Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What should happen if the highlighted text changes? #224

Open
bokand opened this issue Apr 4, 2023 · 3 comments
Open

What should happen if the highlighted text changes? #224

bokand opened this issue Apr 4, 2023 · 3 comments

Comments

@bokand
Copy link
Collaborator

bokand commented Apr 4, 2023

e.g. Jake's example from #217 (comment): https://output.jsbin.com/vohimek/3/quiet

Chrome's behavior of keeping parts of the highlight across a word is clearly poor but the spec doesn't specify any behavior here.

We could specify:

  1. If any of the highlighted text changes, remove the highlight.
  2. Keep the highlight if, and only if, the changed text still matches the directive (i.e. the wildcard text in the middle of a range-based directive). Likely to be computationally intensive as we'd have to re-perform the search on each DOM mutation. We could optimize it since we don't have to search the whole document but this is implementation intensive.
  3. Keep highlighting the same range

I slightly lean to option 1 since it means the highlighted text is always what was searched for but wouldn't be opposed to option 3.

@annevk
Copy link
Collaborator

annevk commented Apr 5, 2023

What does 3 mean? Ideally this matches :target behavior I think, which I think argues for 2.

@bokand
Copy link
Collaborator Author

bokand commented Apr 5, 2023

3 means that the highlight is created with a Range (node A offset a, node B offset b) and we continue to highlight that range even if the content within it changes, though we'd have to decide what to do at range boundaries (node A or B are removed or their content changed to make the offset invalid)

2 is the most intuitive and is consistent with :target but would require, for each highlight, to observe a (potentially-large) subtree for mutations (I guess option 1 also has this) and re-perform the text search each time. I don't have a great intuition for how expensive a mutation observer can be or how often mutations might fire but that makes me a bit worried. The text search could be performed over a limited subtree (e.g. the :target which is the first common ancestor of the starting and ending node of the highlight) and without context but that could still be a non-trivial tree walk.

@annevk
Copy link
Collaborator

annevk commented Apr 5, 2023

Fair, @megangardner thoughts about this?

Presumably we have this problem with find-in-page and various other features as well. Perhaps consistency with them should be sought rather than :target. Hmm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants