Bug 116859

Summary: REGRESSION (r143070): Overflow:scroll content does not get clipped properly when the parent box has CSS3 filter on.
Product: WebKit Reporter: alan <zalan>
Component: Layout and RenderingAssignee: alan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, esprehn+autocc, glenn, rniwa, simon.fraser
Priority: P2 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 117336    
Bug Blocks:    
Attachments:
Description Flags
test case
none
Patch
none
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
none
Patch none

alan
Reported 2013-05-28 03:47:48 PDT
the following code produces a broken content where the overflown content does not get clipped at the edge of the box. <style> .box { overflow: scroll; height: 150px; width: 150px; border: solid 1px red; white-space: nowrap; } .shadow { -webkit-filter: drop-shadow(100px 100px 1px blue); } </style> <div class="shadow"> <div class="before box">long text here........</div> </div>
Attachments
test case (935 bytes, text/html)
2013-05-28 03:48 PDT, alan 		no flags
Details
Patch (6.33 KB, patch)
2013-05-29 12:42 PDT, alan 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (542.12 KB, application/zip)
2013-05-29 19:26 PDT, Build Bot 		no flags
Details
Patch (9.06 KB, patch)
2013-06-03 04:38 PDT, alan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
alan
Comment 1 2013-05-28 03:48:27 PDT
Created attachment 203038 [details] test case
alan
Comment 2 2013-05-28 03:49:09 PDT
<rdar://problem/13845888>
alan
Comment 3 2013-05-29 12:42:15 PDT
Created attachment 203260 [details] Patch
Build Bot
Comment 4 2013-05-29 19:26:45 PDT
Comment on attachment 203260 [details] Patch Attachment 203260 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/742271 New failing tests: fast/dom/location-new-window-no-crash.html
Build Bot
Comment 5 2013-05-29 19:26:47 PDT
Created attachment 203300 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
alan
Comment 6 2013-05-30 03:27:08 PDT
Comment on attachment 203260 [details] Patch EWS:Unable to pass tests without patch (tree is red?) looks unrelated.
Simon Fraser (smfr)
Comment 7 2013-05-31 14:54:37 PDT
Comment on attachment 203260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203260&action=review > Source/WebCore/rendering/RenderLayer.cpp:3921 > + // fragment should paint. If the parent's filter dictates full repaint to ensure proper filter effect, > + // use the overflow clip as dirty rect, instead of no clipping. It maintains proper clipping for overflow::scroll. > + LayoutRect paintDirtyRect = localPaintingInfo.paintDirtyRect; > + if (!paintingInfo.clipToDirtyRect && renderer()->hasOverflowClip()) { > + // We can turn clipping back by requesting full repaint for the overflow area. > + localPaintingInfo.clipToDirtyRect = true; > + paintDirtyRect = selfClipRect(); > + } Does this do the correct thing if the overflow is on a descendant of the filtered element? Should that have its own testcase? > LayoutTests/css3/filters/filter-repaint-shadow-on-parent-and-overflow-scroll-expected.html:9 > + white-space: nowrap; Is this relevant? > LayoutTests/css3/filters/filter-repaint-shadow-on-parent-and-overflow-scroll-expected.html:11 > + -webkit-filter: blur(0px); I would change this to a non-zero blur, because I could imagine us optimizing hasFilterThatMovesPixels() to return false for zero-radius blurs.
alan
Comment 8 2013-06-03 04:38:38 PDT
Created attachment 203574 [details] Patch
alan
Comment 9 2013-06-03 04:42:59 PDT
(In reply to comment #7) > (From update of attachment 203260 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203260&action=review > > > Source/WebCore/rendering/RenderLayer.cpp:3921 > > + // fragment should paint. If the parent's filter dictates full repaint to ensure proper filter effect, > > + // use the overflow clip as dirty rect, instead of no clipping. It maintains proper clipping for overflow::scroll. > > + LayoutRect paintDirtyRect = localPaintingInfo.paintDirtyRect; > > + if (!paintingInfo.clipToDirtyRect && renderer()->hasOverflowClip()) { > > + // We can turn clipping back by requesting full repaint for the overflow area. > > + localPaintingInfo.clipToDirtyRect = true; > > + paintDirtyRect = selfClipRect(); > > + } > > Does this do the correct thing if the overflow is on a descendant of the filtered element? Should that have its own testcase? The original bug is actually about the overflow:scroll on the descendant of the filtered element. So the patch covers that case (too). The reftest was a little bit misleading and I changed it to cover both cases. > > > LayoutTests/css3/filters/filter-repaint-shadow-on-parent-and-overflow-scroll-expected.html:9 > > + white-space: nowrap; > > Is this relevant? No it is not (was just to force the scrollbar), I removed it. > > > LayoutTests/css3/filters/filter-repaint-shadow-on-parent-and-overflow-scroll-expected.html:11 > > + -webkit-filter: blur(0px); > > I would change this to a non-zero blur, because I could imagine us optimizing hasFilterThatMovesPixels() to return false for zero-radius blurs. Good point. I changed that too. Thanks!
WebKit Commit Bot
Comment 10 2013-06-03 06:37:32 PDT
Comment on attachment 203574 [details] Patch Clearing flags on attachment: 203574 Committed r151110: <http://trac.webkit.org/changeset/151110>
WebKit Commit Bot
Comment 11 2013-06-03 06:37:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.