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

Highlight pseudos use the forced-color status of their originating element #34223

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 26, 2022

Per the resolution in [1], forced-color-adjust should not be one of the
supported properties [2] in highlight pseudos, and the forced-color
state of highlights should be taken from the originating element. This
change updates the HighlightInheritance implementation to match the
resolution.

There are a few parts to this:

  • css_properties.json5 is updated so that forced-color-adjust is no
    longer a valid property for highlight pseudos. But, since shipping
    this change for ::selection may cause compatibility issues, we
    introduce a new valid_for_highlight_legacy parameter that maintains
    the old behavior and we update valid_for_highlight, which will be used
    only for highlights with the new inheritance model enabled, to use the
    new behavior.
  • This uncovered an older bug where DetermineValidPropertyFilter wasn't
    using any filter at all for ::highlight(), ::spelling-error, and
    ::grammar-error. This is now fixed.
  • StyleResolver::ApplyInheritance is updated to propagate forced-colors
    status from the originating element to the corresponding highlight
    pseudo.

The new test is disabled on Mac/Linux due to some small differences
in text decoration painting between those from highlight pseudos
and from non-highlight rules, unrelated to the contents of this change.

[1] w3c/csswg-drafts#7264 (comment)
[2] https://drafts.csswg.org/css-pseudo-4/#highlight-styling

Bug: 1309835, 1024156
Change-Id: I732afd24bbba2619a6718270faa0a1d2fcb98512
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3665644
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Delan Azabani <[email protected]>
Commit-Queue: Dan Clark <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1009639}

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-3665644 branch 2 times, most recently from 61a4019 to 98d285d Compare May 31, 2022 22:56
…ement

Per the resolution in [1], forced-color-adjust should not be one of the
supported properties [2] in highlight pseudos, and the forced-color
state of highlights should be taken from the originating element. This
change updates the HighlightInheritance implementation to match the
resolution.

There are a few parts to this:
- css_properties.json5 is updated so that forced-color-adjust is no
  longer a valid property for highlight pseudos. But, since shipping
  this change for ::selection may cause compatibility issues, we
  introduce a new valid_for_highlight_legacy parameter that maintains
  the old behavior and we update valid_for_highlight, which will be used
  only for highlights with the new inheritance model enabled, to use the
  new behavior.
- This uncovered an older bug where DetermineValidPropertyFilter wasn't
  using any filter at all for ::highlight(), ::spelling-error, and
  ::grammar-error. This is now fixed.
- StyleResolver::ApplyInheritance is updated to propagate forced-colors
  status from the originating element to the corresponding highlight
  pseudo.

The new test is disabled on Mac/Linux due to some small differences
in text decoration painting between those from highlight pseudos
and from non-highlight rules, unrelated to the contents of this change.

[1] w3c/csswg-drafts#7264 (comment)
[2] https://drafts.csswg.org/css-pseudo-4/#highlight-styling

Bug: 1309835, 1024156
Change-Id: I732afd24bbba2619a6718270faa0a1d2fcb98512
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3665644
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Delan Azabani <[email protected]>
Commit-Queue: Dan Clark <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1009639}
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3665644 branch from 98d285d to 7208741 Compare June 1, 2022 16:24
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 2350576 into master Jun 1, 2022
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-3665644 branch June 1, 2022 16:32
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.

3 participants