`text-decoration: underline` is not applied to web component

• Component: CSS
• Status: RESOLVED FIXED

[Jeroen Zwartepoorte]

See https://codepen.io/jpzwarte/pen/VwpdPqJ?editors=1010

This rule is not working in Safari (14.1.1 and STP125):

      :host([fill]:hover) {
        text-decoration: underline;
      }

It works fine in Chrome and FF.

[Jeroen Zwartepoorte]

Another example, even more simple: https://codepen.io/Westbrook/pen/dyvKvXP?editors=1010

[Radar WebKit Bug Importer]

<rdar://problem/78987286>

[Antti Koivisto]

Looks like effective text decoration don't pass correctly to shadow tree.

[Antti Koivisto]

Created attachment 430818 [details]
patch

[Antti Koivisto]

Created attachment 430819 [details]
patch

[Ryosuke Niwa]

Comment on attachment 430819 [details]
patch

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

> Source/WebCore/ChangeLog:12
> +        Test case by Jeroen Zwartepoorte.

Maybe put this in the change log for the layout test?

> Source/WebCore/style/StyleAdjuster.cpp:154
> +    auto isAtUserAgentShadowBoundary = [&] {
> +        if (!element)
> +            return false;
> +        auto* parentNode = element->parentNode();
> +        return parentNode && parentNode->isUserAgentShadowRoot();
> +    }();

Hm... I feel like we should be checking the slot assignment here
but I suppose we don't let a node inside a UA shadow root assigned to an author defined shadow tree,
and we don't really set text decoration within UA shadow tree so it's probably okay.
It might be still good to add a comment in the code explaining why this is okay though.

> Source/WebCore/style/StyleAdjuster.cpp:166
> +    default:
> +        return true;

I think putting this return true outside switch would read better.

[Antti Koivisto]

> Hm... I feel like we should be checking the slot assignment here
> but I suppose we don't let a node inside a UA shadow root assigned to an
> author defined shadow tree,
> and we don't really set text decoration within UA shadow tree so it's
> probably okay.
> It might be still good to add a comment in the code explaining why this is
> okay though.

I don't think we should really have this test at all. UA shadow trees that don't want to inherit the effective text-decoration should simply set it themselves. That is a riskier change though and we probably don't have good test coverage.

[Ryosuke Niwa]

(In reply to Antti Koivisto from comment #7)
> > Hm... I feel like we should be checking the slot assignment here
> > but I suppose we don't let a node inside a UA shadow root assigned to an
> > author defined shadow tree,
> > and we don't really set text decoration within UA shadow tree so it's
> > probably okay.
> > It might be still good to add a comment in the code explaining why this is
> > okay though.
> 
> I don't think we should really have this test at all. UA shadow trees that
> don't want to inherit the effective text-decoration should simply set it
> themselves. That is a riskier change though and we probably don't have good
> test coverage.

What do you mean by "simply set it"? It's not possible to get rid of text decorations, right? -webkit-text-decorations-in-effect is a readonly CSS property.

[Antti Koivisto]

Created attachment 430822 [details]
patch

[Antti Koivisto]

> What do you mean by "simply set it"? It's not possible to get rid of text
> decorations, right? -webkit-text-decorations-in-effect is a readonly CSS
> property.

Yeah, I suppose it is bit hard to get rid of it without an extension that makes the property directly available in UA style.

[Antti Koivisto]

Created attachment 430825 [details]
patch

[EWS]

Committed r278602 (238591@main): <https://commits.webkit.org/238591@main>

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