Bug 78759

Summary: REGRESSION (r107836): fast/box-shadow/spread.html and fast/writing-mode/box-shadow-*.html tests failing
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: harrison, hyatt, tony
Priority: P1 Keywords: InRadar, LayoutTestFailure, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 78755    
Attachments:
Description Flags
Refine checks in boxShadowShouldBeAppliedToBackground() simon.fraser: review+

Description mitz 2012-02-15 16:22:25 PST
fast/box-shadow/spread.html and fast/writing-mode/box-shadow-*.html tests failing after r107836, the fix for bug 78728.
Comment 1 mitz 2012-02-15 16:48:06 PST
Created attachment 127276 [details]
Refine checks in boxShadowShouldBeAppliedToBackground()
Comment 2 Simon Fraser (smfr) 2012-02-15 18:00:44 PST
Comment on attachment 127276 [details]
Refine checks in boxShadowShouldBeAppliedToBackground()

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

> Source/WebCore/rendering/InlineFlowBox.cpp:1086
> +    return (!hasFillImage && !renderer()->style()->hasBorderRadius()) || (!prevLineBox() && !nextLineBox()) || !parent();

It's hard to understand all the conditions in this line. Perhaps an explanatory comment would help.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:2623
> +    if (lastBackgroundLayer->image() && style()->hasBorderRadius())
> +        return false;
> +
> +    if (inlineFlowBox && !inlineFlowBox->boxShadowCanBeAppliedToBackground(*lastBackgroundLayer))
> +        return false;

Why do we only check the last fill layer?
Comment 3 mitz 2012-02-15 19:25:28 PST
(In reply to comment #2)
> (From update of attachment 127276 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127276&action=review
> 
> > Source/WebCore/rendering/InlineFlowBox.cpp:1086
> > +    return (!hasFillImage && !renderer()->style()->hasBorderRadius()) || (!prevLineBox() && !nextLineBox()) || !parent();
> 
> It's hard to understand all the conditions in this line. Perhaps an explanatory comment would help.
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:2623
> > +    if (lastBackgroundLayer->image() && style()->hasBorderRadius())
> > +        return false;
> > +
> > +    if (inlineFlowBox && !inlineFlowBox->boxShadowCanBeAppliedToBackground(*lastBackgroundLayer))
> > +        return false;
> 
> Why do we only check the last fill layer?

That’s the layer that will be casting the shadow, if any.
Comment 4 mitz 2012-02-15 19:32:39 PST
Committed <http://webkit.org/r107874>.
Comment 5 David Harrison 2012-02-23 13:03:31 PST
rdar://10915520