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

CSS highlight pseudos: fix ‘currentColor’ resolution and painting #46968

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jul 2, 2024

When ‘color’ is set to ‘currentColor’ in highlight pseudos, the color
(and any other properties that use ‘currentColor’) needs to be taken
from the ‘currentColor’ of the next active highlight overlay below.

Our impl was reworked significantly in CL:5332250 and CL:5368297, and
this patch improves on that by:

• updating HighlightPaintingStyle and ResolveColorsFromPreviousLayer
to let ‘background-color’ participate in ‘currentColor’ resolution

• fixing ResolveColorsFromPreviousLayer to always read ‘currentColor’,
not the property being resolved, from the previous layer

• making PaintHighlightOverlays pass the correct ‘currentColor’ for
the purposes of painting ‘text-shadow’

• extending PaintHighlightOverlays to paint ‘background-color’ and
‘text-shadow’ by as little as one part at a time, if affected by
‘currentColor’, while keeping parts merged where possible

• removing a stray LineRelativeRect::Unite call that made ::selection
parts fail to clip their decorations to the part

This patch adds eleven new tests with overlapping highlights and a
variety of properties set to ‘currentColor’:

• 001.html tests ‘color’ only (full ::selection)
• 001a.html tests ‘color’ only (partial ::selection)
• 002.html tests ‘color’ and ‘background-color’ (full ::selection)
• 002a.html tests ‘color’ and ‘background-color’ (partial ::selection)
• 002b.html tests ‘background-color’ only (full ::selection)
• 003.html tests ‘color’ and ‘text-decoration’ (full ::selection)
• 003a.html tests ‘color’ and ‘text-decoration’ (partial ::selection)
• 003b.html tests ‘text-decoration’ only (full ::selection)
• 004.html tests ‘color’ and ‘text-shadow’ (full ::selection)
• 004a.html tests ‘color’ and ‘text-shadow’ (partial ::selection)
• 004b.html tests ‘text-shadow’ only (full ::selection)

Bug: 339298411
Change-Id: I2e42a0d655683e76daf507aee3e34085f5eb080b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5644614
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Delan Azabani <dazabani@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1322551}

When ‘color’ is set to ‘currentColor’ in highlight pseudos, the color
(and any other properties that use ‘currentColor’) needs to be taken
from the ‘currentColor’ of the next active highlight overlay below.

Our impl was reworked significantly in CL:5332250 and CL:5368297, and
this patch improves on that by:

• updating HighlightPaintingStyle and ResolveColorsFromPreviousLayer
  to let ‘background-color’ participate in ‘currentColor’ resolution

• fixing ResolveColorsFromPreviousLayer to always read ‘currentColor’,
  not the property being resolved, from the previous layer

• making PaintHighlightOverlays pass the correct ‘currentColor’ for
  the purposes of painting ‘text-shadow’

• extending PaintHighlightOverlays to paint ‘background-color’ and
  ‘text-shadow’ by as little as one part at a time, if affected by
  ‘currentColor’, while keeping parts merged where possible

• removing a stray LineRelativeRect::Unite call that made ::selection
  parts fail to clip their decorations to the part

This patch adds eleven new tests with overlapping highlights and a
variety of properties set to ‘currentColor’:

• 001.html tests ‘color’ only (full ::selection)
• 001a.html tests ‘color’ only (partial ::selection)
• 002.html tests ‘color’ and ‘background-color’ (full ::selection)
• 002a.html tests ‘color’ and ‘background-color’ (partial ::selection)
• 002b.html tests ‘background-color’ only (full ::selection)
• 003.html tests ‘color’ and ‘text-decoration’ (full ::selection)
• 003a.html tests ‘color’ and ‘text-decoration’ (partial ::selection)
• 003b.html tests ‘text-decoration’ only (full ::selection)
• 004.html tests ‘color’ and ‘text-shadow’ (full ::selection)
• 004a.html tests ‘color’ and ‘text-shadow’ (partial ::selection)
• 004b.html tests ‘text-shadow’ only (full ::selection)

Bug: 339298411
Change-Id: I2e42a0d655683e76daf507aee3e34085f5eb080b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5644614
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Delan Azabani <dazabani@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1322551}
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 merged commit 95041c3 into master Jul 3, 2024
21 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-5644614 branch July 3, 2024 04:04
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

3 participants