Open Bug 1727771 Opened 3 years ago Updated 6 days ago

New wpt failures in /css/css-pseudo/selection-decoration-p3.html

Categories

(Core :: Layout: Text and Fonts, defect)

defect

Tracking

()

People

(Reporter: mozilla.org, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [wpt])

Syncing wpt PR 29025 found new untriaged test failures in CI

Tests Affected

New Tests That Don't Pass

/css/css-pseudo/selection-decoration-p3.html: FAIL (Chrome: FAIL, Safari: PASS)

CI Results

Gecko CI (Treeherder)
GitHub PR Head

Notes

These updates will be on mozilla-central once bug 1711702 lands.

Note: this bug is for tracking fixing the issues and is not
owned by the wpt sync bot.

This bug is linked to the relevant tests by an annotation in
https://github.com/web-platform-tests/wpt-metadata. These annotations
can be edited using the wpt interop dashboard
https://jgraham.github.io/wptdash/

If this bug is split into multiple bugs, please also update the
annotations, otherwise we are unable to track which wpt issues are
already triaged. Resolving as duplicate or closing this issue should
be cause the bot to automatically update or remove the annotation.

It looks like the failure here is about the color used to paint the text decoration of the selected range.

Severity: -- → S3
Component: Layout: Generated Content, Lists, and Counters → Layout: Text and Fonts

Hmm, the ::selection style only changes background - why should that affect the color of the text decoration?
https://github.com/web-platform-tests/wpt/pull/29025/files#diff-fcb014dda201690bc307e8e9815a34c7d8d1ff6bf70aba4183e225118a8f7836

Flags: needinfo?(jfkthame)

It looks like the test's expectation is that ::selection will change the color, and then the decoration within the selection should also get that new color (if I'm understanding https://drafts.csswg.org/css-pseudo-4/#highlight-painting correctly; though AFAICT we don't implement that). But I don't think this expectation -- that ::selection necessarily changes the current color -- is true. How selections are painted (by default) varies between platforms. So this test may actually be bogus.

Flags: needinfo?(jfkthame)

It looks like the test's expectation is that ::selection will change the color, ...

Right, but I don't see anything in the spec requiring that, so the test indeed looks bogus.

though AFAICT we don't implement that

We do implement ::selection and support a few properties on that, including color and background-color.
The test actually works fine although you can't tell because it renders exactly as the unselected case because they use transparent as the background-color, but if you change that to blue or whatever then the selection shows up.

I really don't see how a text decoration that has an explicitly specified text-decoration-color: green should ever turn up black though.

(Besides, not specifying color explicitly on ::selection means it can be pretty much any color. There's no mandatory spec requirement that it should be black or the color of the originating element. It looks like our selection colors comes from the theme, assuming that's not dead code.)

So, the first paragraph of the spec here is problematic. I suspect the assumption is that the child frames are painting the text decoration instead of the parent the text-decoration style was specified for. That's incompatible with how text-decoration is specified to work.
Consider for example:

data:text/html,<!doctype html><u>A<sup>B</sup><big>C</big><small>D</small></u>

Note that the text-decoration spec explicitly calls out this case, and we render it correctly as illustrated in the spec, whereas Chrome/Safari does not.

It seems that whoever wrote the CSS Highlight spec made the bogus assumption that CSS text decorations are painted correctly in Chrome and designed the Highlight painting accordingly. This is rather worrisome. I think we need to reject the CSS Highlight spec as it's currently written (at least for the text-decoration part, I haven't looked deeply into the other parts).

Here's the Chrome bug (reported by @jfkthame in 2019) regarding their non-compliant text-decoration implementation.

Delan, ^, fyi as you're looking into this stuff iiuc.

Flags: needinfo?(dazabani)

Thanks Emilio!

(In reply to Mats Palmgren (:mats) from comment #4)

I really don't see how a text decoration that has an explicitly specified text-decoration-color: green should ever turn up black though.

(In reply to Jonathan Kew (:jfkthame) from comment #3)

It looks like the test's expectation is that ::selection will change the color, and then the decoration within the selection should also get that new color (if I'm understanding https://drafts.csswg.org/css-pseudo-4/#highlight-painting correctly; though AFAICT we don't implement that).

More or less. The highlighted parts of decorations that came from the unhighlighted originating text must be recolored to the color of the topmost active highlight. This is regardless of whether the author has changed any highlight colors, so for example, if the UA defaults for ::selection are white on blue, then selecting some text should recolor all of its originating decorations to white in the selected parts.

The intent behind this rule is to improve contrast of decorations lifted from unhighlighted content, avoiding situations like dark decorations becoming invisible when ::selection styles are white on black. It doesn’t apply to other highlights though, such as ::spelling-error and ::grammar-error, so red and green squiggly lines retain their colors.

But I don't think this expectation -- that ::selection necessarily changes the current color -- is true. How selections are painted (by default) varies between platforms. So this test may actually be bogus.

So the main::selection rule was based on my interpretation of the last sentence (and note) in #highlight-cascade: declaring background-color should suppress any UA default used-value highlight colors (background-color and color), so I figured that main::selection’s color would be initial (because none of the ancestor ::selection styles yield a cascaded value), just like it is for main itself.

I wrote the selection-decoration-* tests in a bit of a hurry, so in hindsight, even if my reasoning here was valid, I can see I should have written this test more clearly and avoided these load-bearing assumptions. Would it be enough if we added color:black to both rules, seeing as these tests don’t involve any decorations propagated from originating ancestors (see below)?

(In reply to Mats Palmgren (:mats) from comment #5)

So, the first paragraph of the spec here is problematic. I suspect the assumption is that the child frames are painting the text decoration instead of the parent the text-decoration style was specified for.

Yes, speaking only with my impl hat on, our inheritance-hack approach to propagating decorations is bad and responsible for lots of bugs (553174, 1172623, 1008951). I can’t speak for the spec authors, but my best guess is that originating ancestor decorations had just not been considered yet, like so many other things that no one thought about until the first impl (#3932, #6022, #6264, #6386).

Can you please post a CSSWG issue with your concerns? In the meantime, I think none of the highlight decoration tests have decorations that weren’t specified directly on the originating element (or a highlight pseudo), so the tests should otherwise still be passable by any impl.

Flags: needinfo?(dazabani)

Update: I’ve opened wpt#30283 to make these tests clearer, and hopefully fix several wpt.fyi false positives too.

Update (cc :mats): I’ve opened csswg-drafts#6829 to address whether highlights have decoration propagation, and if so, how they interact.

You need to log in before you can comment on or make changes to this bug.