[webkit-reviews] review granted: [Bug 36783] Update of fixed elements is not made correctly when the page has been scrolled : [Attachment 53741] patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Benjamin Poulain
<benjamin.poulain at nokia.com>'s request for review:
Bug 36783: Update of fixed elements is not made correctly when the page has
been scrolled
https://bugs.webkit.org/show_bug.cgi?id=36783

Attachment 53741: patch
https://bugs.webkit.org/attachment.cgi?id=53741&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 4e75c14..79a3bd6 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,24 @@
> +2010-04-19  Benjamin Poulain  <ikipou at gmail.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Update of fixed elements is not made correctly when the page has
been scrolled
> +	   https://bugs.webkit.org/show_bug.cgi?id=36783
> +
> +	   Add the method RenderLayer::updateRepaintRectsAfterScroll() to
update
> +	   the repaint rect of all fixed tree after scroll.

This doesn't clearly state what the problem was (stale cached on RenderLayers),
and what
you did to fix it.

> diff --git a/WebCore/rendering/RenderLayer.cpp
b/WebCore/rendering/RenderLayer.cpp
> index f6c3d80..c8a398e 100644
> --- a/WebCore/rendering/RenderLayer.cpp
> +++ b/WebCore/rendering/RenderLayer.cpp
> @@ -373,6 +373,18 @@ void RenderLayer::computeRepaintRects()
>      m_outlineBox = renderer()->outlineBoundsForRepaint(repaintContainer);
>  }
>  
> +void RenderLayer::updateRepaintRectsAfterScroll(bool fixed)
> +{
> +    if (fixed || renderer()->style()->position() == FixedPosition) {
> +	   computeRepaintRects();
> +	   fixed = true;
> +    } else if (renderer()->hasTransform())
> +	   return;

I would like to see a comment here. Something like
 // Transforms act as fixed position containers, so nothing inside a
transformed element can be fixed relative to the viewport.

But now I think about this, what happens with:

<div style="position: fixed>
  <div style="-webkit-transform: translate(10px, 10px)">
    <div style="position: fixed>
    </div>
  </div>
</div>
?
r=me if that last testcase works.