Cannot style ::selection for a flex container

• Component: CSS
• Status: RESOLVED FIXED

See the repro here: https://jsfiddle.net/gs791rx4/8/

Selecting "some text" will show a red backgrounded selection on Chrome but a default selection background on Safari.

[Alexey Proskuryakov]

<style>
.example {
  display: flex;
}

.example::selection {
  background: red !important;
}
</style>
<div class="example">
  some text
</div>

[Radar WebKit Bug Importer]

<rdar://problem/61117620>

[Tyler Wilcock]

Did a little digging into this one.  From what I can tell, this is happening because the TextRender's parent is sometimes set as anonymous when it maybe shouldn't be.

https://github.com/WebKit/webkit/blob/master/Source/WebCore/rendering/TextPaintStyle.cpp#L161

calls here:

https://github.com/WebKit/webkit/blob/master/Source/WebCore/rendering/RenderText.h#L303#L306

eventually calling here, and shortcircuiting `nullptr` in the `isAnonymous()` check:

https://github.com/WebKit/webkit/blob/master/Source/WebCore/rendering/RenderElement.cpp#L1407#L1410

Also of note, this behavior is not specific to `display: flex`.  See this repro with `display: block` and the introduction of a sibling div.  Again, "some text" should be highlighted red, assuming Chrome and Firefox are correct and our behavior is not.  Removing the sibling div results in the correct behavior.

https://jsfiddle.net/dwuzsmft/

Going to do some more investigation.

[Tyler Wilcock]

^^^ RenderText, not TextRender.  Also, I thought those were permalinks, but they aren't.  Here are those three Github URLs as permalinks (the old ones are already broken):

https://github.com/WebKit/webkit/blob/067ffd731ff6bf0b868e4cb284e2bfa90ee6de2d/Source/WebCore/rendering/TextPaintStyle.cpp#L161

https://github.com/WebKit/webkit/blob/02a0c958678dbba8abb8c83ee888cdbbf66f2a25/Source/WebCore/rendering/RenderText.h#L306

https://github.com/WebKit/webkit/blob/c8e58f74b5c21bfd9e4dd55f31e04cb1d58b5143/Source/WebCore/rendering/RenderElement.cpp#L1425#L1428

[Tyler Wilcock]

Created attachment 396256 [details]
Patch

[Tyler Wilcock]

Created attachment 396257 [details]
Patch

[Tyler Wilcock]

I think this should do it.  The TLDR is that RenderText unconditionally checks its direct parent for pseudostyles, but the direct parent of text nodes/runs is often anonymous, and pseudostyles are associated with elements of the DOM (i.e., non-anonymous).  My proposed patch changes RenderText look for pseudostyles by ascending the tree until a non-anonymous renderer is found.  Should be ready for review.

I added two test cases.  I'm running the GTK port of WebKit, so test expectations were added for LayoutTests/platform/gtk.  According to this (https://trac.webkit.org/wiki/CreatingLayoutTests) wiki article, I also need to add test expectations for other platforms.

What is the best way to do this?  I'm not seeing anything on the Builders page that shows newly generated test expectations as the previous article suggests, but perhaps I'm missing it.

[Tyler Wilcock]

Created attachment 399934 [details]
Patch

[Tyler Wilcock]

Rebased onto latest master -- tests are still passing.

[Antti Koivisto]

Comment on attachment 399934 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399934&action=review

> Source/WebCore/rendering/RenderObject.cpp:170
> +    RenderElement* ancestor = parent();

Could use auto*

> Source/WebCore/rendering/RenderText.h:287
> +    if (RenderElement* ancestor = firstNonAnonymousAncestor())

Here too

> Source/WebCore/rendering/RenderText.h:294
> +    if (RenderElement* ancestor = firstNonAnonymousAncestor())

etc

[Tyler Wilcock]

Created attachment 399968 [details]
Patch

[Tyler Wilcock]

Thanks for the review, Antti.  I applied your suggestions in this latest patch and tests are again green.  I think my addition of a new patch nullified your r+ -- could you take another look, please?

[EWS]

Committed r262049: <https://trac.webkit.org/changeset/262049>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399968 [details].

[Ryan Haddad]

Added test baselines for macOS and iOS in r262059