Bug 226724

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

<rdar://problem/78987286>

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

Created attachment 430818 [details] patch

Created attachment 430819 [details] patch

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.

> 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.

(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.

Created attachment 430822 [details] patch

> 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.

Created attachment 430825 [details] patch

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].