[webkit-reviews] review canceled: [Bug 72852] Scroll position is lost after hide/show element : [Attachment 132823] patch_rebaselined_tot

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has canceled Rakesh
<rakesh.kn at motorola.com>'s request for review:
Bug 72852: Scroll position is lost after hide/show element
https://bugs.webkit.org/show_bug.cgi?id=72852

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132823&action=review


(In reply to comment #55)
> Julien, can you please review this patch.

Sorry, the patch slipped through my emails. I redid the testing and found quite
different supports (using the attached test case):
* IE9 doesn't support resetting the scroll offset.
* Opera doesn't clear the scroll position when moved between documents or
inside the same document.
* FF matches our implementation.

It seems like a desirable change but I would like the ChangeLog updated to
underline that we are chosing to match Firefox here.

> Source/WebCore/rendering/RenderLayer.cpp:221
> +    Node* node = m_renderer->node();
> +    if (node && node->isElementNode())
> +	   toElement(node)->setSavedLayerScrollOffset(m_scrollOffset);

This should be guarded by !m_renderer->documentBeingDestroyed() as we don't
want to allocate extra rare data on document destruction.