Bug 209930

Summary:

REGRESSION (iOS 13.4): Inset box-shadow rendering bug

Product:

WebKit

Reporter:

Andrea <andrea.verlicchi>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, changseok, dylan, esprehn+autocc, ews-watchlist, glenn, ilari, kondapallykalyan, me, pdr, simon.fraser, thatguy, webkit-bug-importer, ysuzuki, zalan

Priority:

Keywords:

InRadar

Version:

Safari 13

Hardware:

iPhone / iPad

OS:

iOS 13

URL:

https://codepen.io/smfr/pen/QWbRbmW

Attachments:

Description	Flags
Rendering bug on iOS 13.4	none
Code patch only	none
Patch	none
Patch	none

Description Andrea 2020-04-02 14:35:07 PDT

Created attachment 395301 [details]
Rendering bug on iOS 13.4

Read my post on medium with the attached DEMO on CODEPEN:
https://medium.com/@andrea.verlicchi/inset-box-shadow-rendering-bug-on-safari-ios-13-4-6bf4256a1ee0

This is the content:

Some of my workmates updated their iPhones to iOS 13.4 and they found that Apple introduced a bug in the rendering of boxes with specific CSS rules for box-shadow . We had found the problem on some of our websites, then we created a new Codepen to isolate and identify the problem.

TL;DR: The rendering problem happens when you use a box-shadow rule with an inset value and a large negative spread-radius.

The spread-radius is the fourth length value. Positive values will cause the shadow to expand and grow bigger, negative values will cause the shadow to shrink. If not specified, it will be 0, so the shadow will be the same size as the element. (source: MDN)

Firstly, we created an empty div on Codepen, applied it a size, a background color, and an inset box-shadow with no x/y offset, a 20px blur-radius and a relatively small (-10px) spread-radius.

Small negative blur-radius

The example below uses a blur-radius of -10px on a box 300px large and 300px high.

box-shadow: 0 0 20px -10px black inset;

We tested it both in an older version of iOS Safari 12.4.6 and on iOS Safari 13.4. The rendering appears good in both versions of the browser.

Large negative blur-radius

The example below uses a blur-radius of -150px on a box 300px large and 300px high.

If you put a larger negative number for the spread-radius value, iOS will render the box with a black bar, horizontally positioned depending on the amount of the spread radius you set.

Comment 1 Radar WebKit Bug Importer 2020-04-02 20:39:50 PDT

<rdar://problem/61240653>

Comment 2 Simon Fraser (smfr) 2020-04-02 20:44:19 PDT

Reproduces on macOS too.

Comment 3 me 2020-05-12 17:25:59 PDT

Looks like absolute value of the box-shadow spread is not important, but rather the proportion to the width of the element that the inset box-shadow is applied to. I was able to reproduce this issue with a thinner box and a lower spread radius.

Some additional notes: The colour of the bar is dependent on the colour of the box shadow; alpha values are ignored. The width of the bar is dependent on (but not exactly) the blur radius of the box-shadow. Hope this helps whoever's working on the issue.

Comment 4 me 2020-05-14 16:39:24 PDT

Appears to be a regression of r252689, specifically these two lines:

            LayoutUnit xOffset = 2 * paintRect.width() + std::max<LayoutUnit>(0, shadowOffset.width()) + shadowPaintingExtent + 2 * shadowSpread + LayoutUnit(1);
             LayoutSize extraOffset(xOffset.ceil(), 0);


The previous code was subtracting 2 * shadowSpread, but the patch changed it to adding 2 * shadowSpread. 

Patch attached.

Comment 5 me 2020-05-14 16:40:44 PDT

Created attachment 399427 [details]
Code patch only

Comment 6 me 2020-05-15 00:43:18 PDT

Created attachment 399459 [details]
Patch

Comment 7 Simon Fraser (smfr) 2020-05-15 09:55:34 PDT

Comment on attachment 399459 [details]
Patch

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

> Source/WebCore/rendering/RenderBoxModelObject.cpp:2398
> -            LayoutUnit xOffset = paintRect.width() + std::max<LayoutUnit>(0, shadowOffset.width()) + shadowPaintingExtent + 2 * shadowSpread + LayoutUnit(1);
> +            LayoutUnit xOffset = paintRect.width() + std::max<LayoutUnit>(0, shadowOffset.width()) + shadowPaintingExtent - 2 * shadowSpread + LayoutUnit(1);

The bug is about inset shadows, but this is the Normal shadow code path, so this can't be the right fix.

Comment 8 me 2020-05-15 11:42:18 PDT

Created attachment 399500 [details]
Patch

Comment 9 me 2020-05-15 11:43:13 PDT

Oops, you're right. Fixed.

Comment 10 EWS 2020-11-01 19:50:47 PST

Committed r269242: <https://trac.webkit.org/changeset/269242>

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