Bug 116859 – REGRESSION (r143070): Overflow:scroll content does not get clipped properly when the parent box has CSS3 filter on.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

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>

[zalan]

Created attachment 203038 [details]
test case

[zalan]

<rdar://problem/13845888>

[zalan]

Created attachment 203260 [details]
Patch

[Build Bot]

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]

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

[zalan]

Comment on attachment 203260 [details]
Patch

EWS:Unable to pass tests without patch (tree is red?) 

looks unrelated.

[Simon Fraser (smfr)]

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.

[zalan]

Created attachment 203574 [details]
Patch

[zalan]

(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 on attachment 203574 [details]
Patch

Clearing flags on attachment: 203574

Committed r151110: <http://trac.webkit.org/changeset/151110>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.