[webkit-reviews] review denied: [Bug 105486] Need to re-layout fixed position elements after scale when using settings()->fixedElementsLayoutRelativeToFrame() : [Attachment 186688] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Tien-Ren Chen
<trchen at chromium.org>'s request for review:
Bug 105486: Need to re-layout fixed position elements after scale when using
settings()->fixedElementsLayoutRelativeToFrame()
https://bugs.webkit.org/show_bug.cgi?id=105486

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=186688&action=review


> Source/WebCore/page/FrameView.cpp:2026
> +    if (fixedElementsLayoutRelativeToFrame())
> +	   setViewportConstrainedObjectsNeedLayout();

I don't think this is the right fix for the problem you are seeing. 
visibleContentsResized() is called from ScrollView::updateScrollbars() so that
the view can react to the changes in visible content size due to overflow
controls being created/destroyed.  In your case, you have overlay scrollbars so
the overflow controls do not change the amount of content space or visible
content space.	Thus there's no reason to react to visibleContentsResized since
they haven't actually resized.	I suspect you observe that this patch papers
over the bug you have since it's called frequently and happens to trigger in
the case where you actually do need additional layout.

You need to figure out what is actually changing and why that is not triggering
the layout you need.