[webkit-reviews] review granted: [Bug 101001] Pages with position:fixed elements should still be able to scroll on the scrolling thread : [Attachment 172370] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Beth Dakin
<bdakin at apple.com>'s request for review:
Bug 101001: Pages with position:fixed elements should still be able to scroll
on the scrolling thread
https://bugs.webkit.org/show_bug.cgi?id=101001

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=172370&action=review


> Source/WebCore/page/FrameView.cpp:1853
> +    if (!page)
> +	   return false;

Return true?

> Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm:397
> +	   child->graphicsLayer()->setPosition(position);

Shouldn't this be a syncPosition()?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2665
> +	   // If the fixed element is inside an overflow region, then we can't
scroll it with the ScrollingCoordinator.
> +	   // We will be able to fix this when we get overflow regions
scrolling with the ScrollingCoordinator.

Add a test for this?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2677
> +    // FIXME: We should support sticky position here! And we should
eventuall support fixed/sticky elements
> +    // that are inside non-main frames once we get non-main frames scrolling
with the ScrollingCoordinator.

Would be good to have some tests with fixed/sticky things in subframes.