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

HighlightOverlayPainting: fix buffer confusion and clamping bugs #35290

Merged
merged 1 commit into from Aug 2, 2022

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Aug 1, 2022

Custom highlights and marker-based highlights are defined in terms of
DOM ranges in a Text node. Generated text either has no Text node or
does not derive its content from the text node, so the active ranges
of these highlights should be ignored, but we fail to do so for soft
hyphens (a kind of generated text).

On its own, this can cause spurious paints of things like highlight
backgrounds near soft hyphens if [0,1) is highlighted.

CL:3720688 also changes the logic of ComputeParts, requiring parts to
be added to the result outside the main loop if originating fragment
offsets extend past the first or last highlight edges. For example,
with an originating fragment [a,b) and first highlight edge [c,d)
where a<c<b<d, we add originating part [a,c) before the main loop.

But if a<b<c<d, for example, the originating part should be [a,b), not
[a,c), and this can cause the failures in bug 1346809 and bug 1346810.

This patch fixes both bugs by ignoring marker-based highlight ranges
for all generated text including soft hyphens (like we already do for
ellipsis) and clamping parts to the originating fragment even when
they are added outside the main loop.

Fixed: 1346810, 1346809
Change-Id: If3369bfb1a44ebce190db8fa8477065eeba00d86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3793072
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Delan Azabani <dazabani@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1030541}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3793072 branch 2 times, most recently from a6afaa0 to 4fe8603 Compare August 2, 2022 08:52
Custom highlights and marker-based highlights are defined in terms of
DOM ranges in a Text node. Generated text either has no Text node or
does not derive its content from the text node, so the active ranges
of these highlights should be ignored, but we fail to do so for soft
hyphens (a kind of generated text).

On its own, this can cause spurious paints of things like highlight
backgrounds near soft hyphens if [0,1) is highlighted.

CL:3720688 also changes the logic of ComputeParts, requiring parts to
be added to the result outside the main loop if originating fragment
offsets extend past the first or last highlight edges. For example,
with an originating fragment [a,b) and first highlight edge [c,d)
where a<c<b<d, we add originating part [a,c) before the main loop.

But if a<b<c<d, for example, the originating part should be [a,b), not
[a,c), and this can cause the failures in bug 1346809 and bug 1346810.

This patch fixes both bugs by ignoring marker-based highlight ranges
for all generated text including soft hyphens (like we already do for
ellipsis) and clamping parts to the originating fragment even when
they are added outside the main loop.

Fixed: 1346810, 1346809
Change-Id: If3369bfb1a44ebce190db8fa8477065eeba00d86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3793072
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Delan Azabani <dazabani@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1030541}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants