Bug 237898 – REGRESSION (r282737): `text-shadow` is clipped

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Benoît Rouleau]

In Safari 15.4, the `text-shadow` of an element can be clipped when an absolute-positioned element with `will-change: transform` appears behind it.

This worked fine in Safari 15.3, and continues to work fine in other browsers.

Simple reproduction here: https://codepen.io/benface/pen/yLpNpeG

[Radar WebKit Bug Importer]

<rdar://problem/90320701>

[alan]

This has regressed at r282737.

[alan]

Created attachment 454745 [details]
slightly more visible (test reduction)

[alan]

we are missing some visual overflow here.

[alan]

Created attachment 454772 [details]
Patch

[alan]

Created attachment 454774 [details]
Patch

[Darin Adler]

Comment on attachment 454774 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Inflate the ink overflow rect with the text shadow values (note that here, in the display builder we works with physical coordinates).

works -> work

> Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:146
> +        auto addTextShadowInkOverflow = [&] {

Not sure we need to name this lambda. Could just make it and call it.

> Source/WebCore/rendering/style/RenderStyle.h:508
> +    void getTextShadowHorizontalExtent(LayoutUnit& left, LayoutUnit& right) const { getShadowHorizontalExtent(textShadow(), left, right); }

Should return a struct rather than using “get” style

> Source/WebCore/rendering/style/RenderStyle.h:509
> +    void getTextShadowVerticalExtent(LayoutUnit& top, LayoutUnit& bottom) const { getShadowVerticalExtent(textShadow(), top, bottom); }

Ditto.

[alan]

Created attachment 454784 [details]
Patch

[alan]

(In reply to Darin Adler from comment #7)
> Comment on attachment 454774 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454774&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Inflate the ink overflow rect with the text shadow values (note that here, in the display builder we works with physical coordinates).
> 
> works -> work
> 
> > Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:146
> > +        auto addTextShadowInkOverflow = [&] {
> 
> Not sure we need to name this lambda. Could just make it and call it.
> 
> > Source/WebCore/rendering/style/RenderStyle.h:508
> > +    void getTextShadowHorizontalExtent(LayoutUnit& left, LayoutUnit& right) const { getShadowHorizontalExtent(textShadow(), left, right); }
> 
> Should return a struct rather than using “get” style
> 
> > Source/WebCore/rendering/style/RenderStyle.h:509
> > +    void getTextShadowVerticalExtent(LayoutUnit& top, LayoutUnit& bottom) const { getShadowVerticalExtent(textShadow(), top, bottom); }
> 
> Ditto.
Thanks for the review! Changed the patch a bit.

[EWS]

Committed r291330 (248469@main): <https://commits.webkit.org/248469@main>

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

[Benoît Rouleau]

Wow, fixed within 12 hours of reporting. Thanks everyone!

[alan]

(In reply to Benoît Rouleau from comment #11)
> Wow, fixed within 12 hours of reporting. Thanks everyone!
Thank you for reporting it!

[Darin Adler]

Comment on attachment 454784 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyle.h:508
> +    LayoutBoxExtent textShadowExtent() const { return shadowExtent(textShadow()); }

Oh, that came out nicer than the "get" version!

[alan]

(In reply to Darin Adler from comment #13)
> Comment on attachment 454784 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454784&action=review
> 
> > Source/WebCore/rendering/style/RenderStyle.h:508
> > +    LayoutBoxExtent textShadowExtent() const { return shadowExtent(textShadow()); }
> 
> Oh, that came out nicer than the "get" version!
It really did. I got blinded by get_BoxShadow_HorizontalExtent/get_BoxShadow_VerticalExtent pair a few lines below. Thanks for commenting on it.

[Sam Sneddon [:gsnedders]]

Just to let everyone know, for those who care about Safari, the fix for this has shipped in Safari 15.5.