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-pseudo] [css-highlight-api] defaults and inheritance for ::spelling-error + ::grammar-error + ::highlight #6779

Closed
delan opened this issue Oct 29, 2021 · 5 comments

Comments

@delan
Copy link
Contributor

delan commented Oct 29, 2021

Problem 1: UA default highlight colors

(this problem turned out to effectively be the same as #6774)

If I understand conventional behaviour and #highlight-ua-styles correctly, the intended default behaviour of ::spelling-error and ::grammar-error is to decorate the highlighted text without changing the foreground color.

Similarly, if I understand #6375 correctly, the intended default behaviour of an unstyled ::highlight is to yield no visible change in rendering, or at least not change the foreground color.

But given the following statements…

  1. Highlight pseudo computed styles inherit all properties from the same highlight’s styles in the parent, eventually defaulting to initial values (#highlight-cascade). Used values can differ due to paired cascade ([css-pseudo] clarify paired-cascading behavior #6386).

  2. color:currentColor is the only way to grab the foreground color from lower layers, that is to say, “don’t change the color compared to if this text was unhighlighted” (#highlight-text).

  3. The initial value of color is a UA-defined ordinary color (css-color-3) or CanvasText (css-color-4), not currentColor.

  4. There are no recommended UA default highlight colors for ::spelling-error + ::grammar-error (#highlight-ua-styles), be they magic used colors via paired cascade or just declarations in the UA stylesheet, and UA default highlight colors are forbidden for ::highlight as of [css-highlight-api-1] Specify that custom highlights have no UA styles #6375 #6534 (#default-styles).

…then by default, these three highlight pseudos will effectively reset the foreground color of highlighted text to initial (usually black), which is probably not what we want.

Problem 2: custom highlight inheritance clause

As of #6534, #default-styles says:

A custom highlight pseudo-element inherits the styles of its originating element.

I don’t think this really makes sense, because taken at face value, it essentially clobbers highlight inheritance (#highlight-cascade) in favour of the legacy behaviour (like how ::selection currently works in most impls).

Possible solutions

(see #6774 for solution to problem 1)

I think #highlight-ua-styles and #default-styles should explicitly recommend (or require) UA defaults for these three highlight pseudos that behave like color:currentColor. I believe the defaults should come from magic paired cascade, not UA stylesheet rules, to keep paired cascade as consistent and predictable as possible.

No change is needed for the other highlight color (background-color), because its initial value is transparent.

I think the spec text in Problem 2 should be removed.

@dandclark
Copy link
Contributor

Thanks for pointing this out!

For custom highlights at least, the original intention of the custom highlight #default-styles was indeed for an unstyled ::highlight to yield no visible change in rendering. I had written the spec text with the legacy ::selection behavior in mind, so it doesn't fit with the new #highlight-cascade behavior.

Maybe we could change #default-styles to something like the following, borrowing some language from #highlight-text:

If the author does not specify a custom highlight pseudo-element's color, the text is painted using the color of the next highlight pseudo-element layer below, or if none exists, the colors that would otherwise have been used (those applied by the originating element or an intervening pseudo-element such as ::first-line or ::first-letter). If the author does not specify a custom highlight pseudo-element's background-color, the UA must paint the highlight pseudo-element's background using the value 'transparent'.

delan added a commit to delan/csswg-drafts that referenced this issue Nov 18, 2021
@delan
Copy link
Contributor Author

delan commented Nov 18, 2021

I had written the spec text with the legacy ::selection behavior in mind, so it doesn't fit with the new #highlight-cascade behavior.

Fair enough, that makes sense. I think that proposed wording is a good start, but I think it fails to account for the special relationship between ‘color’ and ‘background-color’ (paired cascade), and suffers from the same problems as “specified by the author” does in #highlight-cascade (#6386).

e6ec959ad9fb3 attempts to word this in a way that builds on the clarifications I’ve proposed in #6665, what do you think?

@dandclark
Copy link
Contributor

Ah, I'd missed that #6665 was also addressing this for custom highlights. It looks great to me!

@delan delan added the Agenda+ label Nov 24, 2021
@astearns astearns removed the Agenda+ label Dec 7, 2021
@astearns
Copy link
Member

astearns commented Dec 7, 2021

We probably don’t need agenda time for this if @frivoal and/or @fantasai could review the PR

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
aarongable pushed a commit to chromium/chromium that referenced this issue Feb 9, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
mattwoodrow pushed a commit to mattwoodrow/wpt that referenced this issue Feb 15, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 26, 2022
…om highlights, a=testonly

Automatic update from web-platform-tests
Highlight API: Don't paint unstyled custom highlights

The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}

--

wpt-commits: 40d5e6da07fc061a6d81f081c3c16eafc10b54ce
wpt-pr: 32759
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this issue Feb 28, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Mar 1, 2022
…om highlights, a=testonly

Automatic update from web-platform-tests
Highlight API: Don't paint unstyled custom highlights

The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}

--

wpt-commits: 40d5e6da07fc061a6d81f081c3c16eafc10b54ce
wpt-pr: 32759
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 8, 2022
…om highlights, a=testonly

Automatic update from web-platform-tests
Highlight API: Don't paint unstyled custom highlights

The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}

--

wpt-commits: 40d5e6da07fc061a6d81f081c3c16eafc10b54ce
wpt-pr: 32759
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Mar 8, 2022
…om highlights, a=testonly

Automatic update from web-platform-tests
Highlight API: Don't paint unstyled custom highlights

The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}

--

wpt-commits: 40d5e6da07fc061a6d81f081c3c16eafc10b54ce
wpt-pr: 32759
@delan
Copy link
Contributor Author

delan commented Oct 6, 2022

#6665 makes the edits for these questions.

@delan delan closed this as completed Oct 6, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The custom highlight API spec [1] currently states that an unstyled
custom highlight should inherit the styles of its originating element.
The Blink implementation follows this.

However, this behavior can lead to some unintuitive results where
unstyled highlights can affect the painting of the originating
element, or paint over other highlights. This issue was pointed out
in [2], and spec changes to fix this are pending editor review.
A goal of these changes are that unstyled custom highlights do not
affect rendering at all.

This change updates the custom highlights implementation to achieve
this by giving them a default 'transparent' foreground and background
color.

Some of our tests were validating the earlier unintuitive behavior
involving overlapping ranges. These are updated, along with the
addition of a new test to validate that a single unstyled highlight
doesn't affect rendering.

[1] https://drafts.csswg.org/css-highlight-api-1/#default-styles
[2] w3c/csswg-drafts#6779

Bug: 1295271
Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#969007}
NOKEYCHECK=True
GitOrigin-RevId: 6ebd27596486309c18c3961b90bfb86cba62d3b8
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

3 participants