Bug 124887 – [CSS Regions] Fix painting when the composited region has overflow:hidden

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mihai Maerean]

The left and top part of the content flowed in the (composited and with overflow:hidden) region is missing.

[Mihai Maerean]

Created attachment 217875 [details]
layout test

[Mihai Maerean]

Created attachment 217876 [details]
layout test expected result

[Mihai Maerean]

Created attachment 218410 [details]
patch

[Alexandru Chiculita]

Comment on attachment 218410 [details]
patch

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

> LayoutTests/compositing/regions/paint-inside-composited-region-overflow-hidden-versus-div-expected.html:30
> +			<div class="content">a b c 1. a b c 2. a b c 3. a b c 4. a b c 5. a b c 6. a b c 7. a b c 8. a b c 9. a b c A. a b c B. a b c C. a b c D. a b c E. a b c F. a b c 0. a b c 1. a b c 2. a b c 3. </div>

Is there any reason for using the "a b c" sequences? I think it makes it hard to follow. Just use plain text.

> Source/WebCore/rendering/RenderLayer.cpp:6768
> +        regionClipRect = backgroundClipRect(clipRectsContext); // Region coordinates.

I think we need to be more explicit about explaining the difference between region / fragment / fragment container.

> Source/WebCore/rendering/RenderLayer.cpp:6778
>          regionClipRect = regionContentBox;

I think that this whole function needs some rework. For example, this extra step will overwrite this line from above "regionClipRect = dirtyRect".

> Source/WebCore/rendering/RenderLayer.cpp:6782
> +        // When the layer of the region is composited, the region receives a GraphicsLayer of its own
> +        // so the clipping coordinates (caused by overflow:hidden) must be relative to the
> +        // GraphicsLayer coordinates in which the region gets painted.
> +        if (!backing()) {

The comment is confusing, especially because you have a !backing() just after it. I think the term "region" is now confusing as you have a "fragmentContainer" and a "fragment". Which one are you refering to as a "region".

Also, there is a phase where all the layers are flatten even though they have a backing. It's used to draw the "drag" image when you drag a portion of the page. That's why I don't think it's a good idea to check just for !backing().

> Source/WebCore/rendering/RenderLayer.cpp:6785
> +            regionClipRect.moveBy(regionOffsetFromRoot); // Screen coordinates.

I don't understand how this could be "screen" coordinates.

[Mihai Maerean]

Comment on attachment 218410 [details]
patch

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

>> Source/WebCore/rendering/RenderLayer.cpp:6782
>> +        if (!backing()) {
> Also, there is a phase where all the layers are flatten even though they have a backing. It's used to draw the "drag" image when you drag a portion of the page. That's why I don't think it's a good idea to check just for !backing().

What else to check for?
Shouldn't this issue (about dragging of a page portion) be tracked as a different bug?

[Mihai Maerean]

Created attachment 218516 [details]
patch. incorporates the feedback.

[Mihai Maerean]

Created attachment 220848 [details]
patch. incorporates the feedback.

The dragged selected text that is composited in a region is not being rendered in Nightly at the moment. I have added https://bugs.webkit.org/show_bug.cgi?id=126755 to track this bug.

[Mihai Maerean]

Created attachment 220849 [details]
patch. incorporates the feedback.

The dragged selected text that is composited in a region is not being rendered in Nightly at the moment. I have added https://bugs.webkit.org/show_bug.cgi?id=126755 to track this bug.

[Build Bot]

Comment on attachment 220849 [details]
patch. incorporates the feedback.

Attachment 220849 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6505188202381312

New failing tests:
fast/regions/overflow-first-and-last-regions-in-container-hidden.html
fast/regions/overflow-scrollable-1.html
fast/regions/overflow-scrollable-3.html
fast/regions/shape-inside/shape-inside-on-first-region-inline-content.html
fast/regions/overflow-scrollable-fit.html
fast/regions/shape-inside/shape-inside-on-first-region-block-content.html
fast/regions/overflow-scrollable-varying-width-1.html

[Build Bot]

Created attachment 220887 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Mihai Maerean]

Created attachment 221145 [details]
patch. incorporates the feedback.

[Mihai Maerean]

please review

[Alexandru Chiculita]

Comment on attachment 221145 [details]
patch. incorporates the feedback.

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

> LayoutTests/compositing/regions/paint-inside-composited-region-overflow-hidden-versus-div-expected.html:4
> +		<title>Expected Result - Bug #124887: [CSS Regions] Fix painting when the composited region has overflow:hidden</title>

nit: Use 4 sapces instead of tabs.

r=me

[Mihai Maerean]

Created attachment 221332 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 221332 [details]
patch for landing

Clearing flags on attachment: 221332

Committed r162115: <http://trac.webkit.org/changeset/162115>