[webkit-reviews] review granted: [Bug 226724] `text-decoration: underline` is not applied to web component : [Attachment 430819] patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted  review:
Bug 226724: `text-decoration: underline` is not applied to web component
https://bugs.webkit.org/show_bug.cgi?id=226724

Attachment 430819: patch

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




--- Comment #6 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 430819
  --> https://bugs.webkit.org/attachment.cgi?id=430819
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.