[webkit-reviews] review denied: [Bug 36783] Update of fixed elements is not made correctly when the page has been scrolled : [Attachment 52963] Patch and pixel test

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied 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 52963: Patch and pixel test
https://bugs.webkit.org/attachment.cgi?id=52963&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

> +void RenderLayer::updateRepaintRectPosition()
> +{
> +    RenderBox* box = renderBox();
> +    FloatPoint absPos = box->localToAbsolute();
> +    absPos.move(box->borderLeft(), box->borderTop());
> +    m_repaintRect.setLocation(IntPoint(absPos.x(), absPos.y()));
> +}

This isn't correct for fixed inside of transformed elements, and some other
configurations.
It should just be removed in favor of computeRepaintRects().

> +void RenderView::updateFixedElementPositions()
> +{
> +    ListHashSet<RenderBox*>::const_iterator end =
positionedObjects()->end();
> +    for (ListHashSet<RenderBox*>::const_iterator it =
positionedObjects()->begin(); it != end; ++it) {
> +	   RenderBox* renderBox = *it;
> +	   if (renderBox->style()->position() != FixedPosition)
> +	       continue;
> +	   renderBox->layer()->updateRepaintRectPosition();
> +    }
> +}

You also need to hit RenderLayers with a fixed ancestor. It may be better to do
something similar to what updateLayerPositions() does, or add a new flag to
updateLayerPositions() and call that.