RESOLVED FIXED 226724
`text-decoration: underline` is not applied to web component 
https://bugs.webkit.org/show_bug.cgi?id=226724
Summary `text-decoration: underline` is not applied to web component
Jeroen Zwartepoorte
Reported 2021-06-07 04:34:04 PDT
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.
Attachments
patch (5.62 KB, patch)
2021-06-08 01:46 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (5.65 KB, patch)
2021-06-08 01:49 PDT, Antti Koivisto 		rniwa: review+
DetailsFormatted DiffDiff
patch (5.76 KB, patch)
2021-06-08 02:36 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (5.76 KB, patch)
2021-06-08 02:57 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jeroen Zwartepoorte
Comment 1 2021-06-07 04:55:06 PDT
Another example, even more simple: https://codepen.io/Westbrook/pen/dyvKvXP?editors=1010
Radar WebKit Bug Importer
Comment 2 2021-06-08 00:08:23 PDT
<rdar://problem/78987286>
Antti Koivisto
Comment 3 2021-06-08 00:27:13 PDT
Looks like effective text decoration don't pass correctly to shadow tree.
Antti Koivisto
Comment 4 2021-06-08 01:46:07 PDT
Created attachment 430818 [details] patch
Antti Koivisto
Comment 5 2021-06-08 01:49:52 PDT
Created attachment 430819 [details] patch
Ryosuke Niwa
Comment 6 2021-06-08 02:20:00 PDT
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
Comment 7 2021-06-08 02:31:03 PDT
> 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
Comment 8 2021-06-08 02:34:15 PDT
(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
Comment 9 2021-06-08 02:36:00 PDT
Created attachment 430822 [details] patch
Antti Koivisto
Comment 10 2021-06-08 02:52:13 PDT
> 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
Comment 11 2021-06-08 02:57:51 PDT
Created attachment 430825 [details] patch
EWS
Comment 12 2021-06-08 05:11:38 PDT
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].
Note You need to log in before you can comment on or make changes to this bug.