Bug 60741 – Incorrect RenderLayer transforms on overflow RTL pages

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Adrienne Walker]

On pages with dir=rtl, compositing, and a horizontal scrollbar, composited layers get incorrect transforms and bounds.  Without any horizontal overflow, the composited case appears identical to the software case.

At the very least, http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp#L444 assumes that the content origin is at 0, so when boundingBoxRect.x() is negative, that calculation ends up with a very large bound.

I will attach some test cases to demonstrate this issue by resizing the window.  Compare the composited cases (with and without a horizontal scrollbar) to the software cases.

[Adrienne Walker]

Created attachment 93380 [details]
Test case: compositing, fixed position layer

[Adrienne Walker]

Created attachment 93381 [details]
Test case: software, fixed position layer

[Adrienne Walker]

Created attachment 93382 [details]
Test case: compositing, absolute position layer

[Adrienne Walker]

Created attachment 93383 [details]
Test case: software, fixed position layer

<rdar://problem/9479308>

[Adrienne Walker]

Created attachment 98402 [details]
Patch

[Adrienne Walker]

I will also mention that I have an in-progress patch that reenables compositing for rtl pages in Chromium.  With that patch in place, these tests pass in Chromium.  I just wanted to fix this underlying general WebKit bug first before committing that code.

[Simon Fraser (smfr)]

Comment on attachment 98402 [details]
Patch

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

> LayoutTests/compositing/rtl/rtl-absolute-overflow-scrolled.html:54
> +    <!-- green square should appear at 50,50 from the top left -->
> +    <div id="layer"></div>

It's better if red shows on failure. Can you put a red box under the green one that would be exposed on failure in all the tests?

[James Robinson]

Comment on attachment 98402 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Fix positioning of the root graphics layer for RTL pages.  The fact
> +        the left side of the page is negative for pages with overflow is not

I think you accidentally a word ("The fact the left side..")

[Adrienne Walker]

Created attachment 98418 [details]
Patch

[Adrienne Walker]

(In reply to comment #10)
> Created an attachment (id=98418) [details]
> Patch

Grammar fixed.  The tests now have non-composited red indicator elements positioned identically underneath the composited green layers.

[Adrienne Walker]

Committed r89632: <http://trac.webkit.org/changeset/89632>

[Simon Fraser (smfr)]

Can you list some real-world sites that are affected by this change?

[Adrienne Walker]

(In reply to comment #13)
> Can you list some real-world sites that are affected by this change?

This could very plausibly trigger on Gmail in Hebrew.  If there's an embedded video in a message, the Gmail fixed position "archive spam delete etc" header could overlap it, become composited, and would then draw entirely incorrectly.

[Adrienne Walker]

Committed r89640: <http://trac.webkit.org/changeset/89640>

[Adrienne Walker]

Reverted r89632 for reason:

Breaks ancestor-overflow-change unexpectedly

Committed r89645: <http://trac.webkit.org/changeset/89645>

[Adrienne Walker]

(In reply to comment #16)
> Reverted r89632 for reason:
> 
> Breaks ancestor-overflow-change unexpectedly
> 
> Committed r89645: <http://trac.webkit.org/changeset/89645>

I thought I had tested this on Safari, but I missed it and it's skipped on Chromium Linux.

I will fix this issue and reupload.  Sorry for the hassle.  :)

[Adrienne Walker]

Created attachment 98514 [details]
Patch

[Adrienne Walker]

(In reply to comment #18)
> Created an attachment (id=98514) [details]
> Patch

Now with fewer test breakages and some additional skipped tests on Chromium Mac.  This ran cleanly on Linux Chromium-GPU and on Snow Leopard Safari.

Root graphics layers need to be adjusted so that their position is zero and their offset is the left of the document.  The previous patch was using the local compositing bounds for the adjustment and not the document rect, which was not quite the right value in all cases.

[Adrienne Walker]

Committed r89687: <http://trac.webkit.org/changeset/89687>

[Adrienne Walker]

Committed r89692: <http://trac.webkit.org/changeset/89692>