Bug 115840 - REGRESSION (r145680): No box shadow rendered on element with positioned child that obscures it
Summary: REGRESSION (r145680): No box shadow rendered on element with positioned child...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
Keywords: InRadar
Depends on:
Reported: 2013-05-08 18:19 PDT by Simon Fraser (smfr)
Modified: 2013-05-10 17:58 PDT (History)
7 users (show)

See Also:

Testcase (684 bytes, text/html)
2013-05-08 18:19 PDT, Simon Fraser (smfr)
no flags Details
Patch (4.34 KB, patch)
2013-05-08 22:10 PDT, Simon Fraser (smfr)
koivisto: review+
Details | Formatted Diff | Diff
Alternative patch (7.32 KB, patch)
2013-05-08 22:27 PDT, Antti Koivisto
simon.fraser: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2013-05-08 18:19:22 PDT
Created attachment 201125 [details]

There should be a box shadow on the parent of the gray box in the testcase, but it's missing.
Comment 1 Radar WebKit Bug Importer 2013-05-08 18:19:48 PDT
Comment 2 Simon Fraser (smfr) 2013-05-08 18:28:39 PDT
Regressed at http://trac.webkit.org/changeset/145680
Comment 3 Simon Fraser (smfr) 2013-05-08 21:57:22 PDT
Ah, there's a code path where paintFillLayer() paints the box shadow.
Comment 4 Simon Fraser (smfr) 2013-05-08 21:58:20 PDT
…which was done to fix bug 78728.
Comment 5 Simon Fraser (smfr) 2013-05-08 22:10:47 PDT
Created attachment 201134 [details]
Comment 6 Antti Koivisto 2013-05-08 22:27:33 PDT
Created attachment 201136 [details]
Alternative patch
Comment 7 Antti Koivisto 2013-05-08 22:31:25 PDT
The simpler approach might be better. I'm not sure if it is semantically correct to consider box shadow in background extent.
Comment 8 Simon Fraser (smfr) 2013-05-08 22:53:20 PDT
Comment on attachment 201136 [details]
Alternative patch

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

> Source/WebCore/rendering/RenderBox.cpp:1151
> +    LayoutRect backgroundRect = pixelSnappedIntRect(style()->boxShadow() ? boxShadowExtent() : borderBoxRect());

I think it's confusing for a function called backgroundPaintedExtent() to return the box shadow extent, unless you rename it to boxDecorationExtent().

This patch also means that you'll always consult box shadows for obscuration, even when the background might be obscured and the shadow drawn separately. So I prefer my patch.
Comment 9 Simon Fraser (smfr) 2013-05-10 17:58:55 PDT