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

fix: rewrite text scanning #1136

Merged
merged 2 commits into from
Apr 17, 2023
Merged

fix: rewrite text scanning #1136

merged 2 commits into from
Apr 17, 2023

Conversation

birtles
Copy link
Member

@birtles birtles commented Apr 12, 2023

Completely overhaul text lookup to cover more cases and be (hopefully) easier to
extend in future (so we can tackle #1130).

Fixes #990.

As mentioned in the discussion for #990, this has a negative performance impact
for most content. This is due to the extra distance calculations performed on
each lookup which is needed to avoid returning a cached position when the
distance has substantially changed.

For some content such as asahi.com, however, there is a net performance
improvement due to the way we handle repeated lookups.

For now this is probably acceptable, but if not, we will need to find a way to
cache the point used as input for the previous lookup and avoid doing distance
calculations if we determine we are close.

@birtles
Copy link
Member Author

birtles commented Apr 12, 2023

This probably needs testing with Safari before merging

@birtles
Copy link
Member Author

birtles commented Apr 12, 2023

Unit tests are failing on Chrome on CI, presumably because we no longer apply the mid-character adjustments to synthesized input text nodes there. That's probably easy enough to add back.

@birtles
Copy link
Member Author

birtles commented Apr 13, 2023

Performance impact is now fixed (in the majority of cases this is faster). Just need to test on Safari which means I need to wait until I can get near a Mac.

@birtles birtles force-pushed the redo-text-scanning branch 3 times, most recently from 958c0a2 to ad46955 Compare April 17, 2023 00:39
Completely overhaul text lookup to cover more cases and (hopefully) be
easier to extend in future (so we can tackle #1130).

Fixes #990.
Fixes #1033.

Also improves performance and allows looking up `user-select: all` text
in Firefox.
@birtles birtles merged commit f8fd8c5 into main Apr 17, 2023
@birtles birtles deleted the redo-text-scanning branch April 17, 2023 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect popup handling in flexbox elements
1 participant