[webkit-reviews] review granted: [Bug 72852] Scroll position is lost after hide/show element : [Attachment 118957] Updated patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted 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 118957: Updated patch
https://bugs.webkit.org/attachment.cgi?id=118957&action=review

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


> Source/WebCore/dom/Element.cpp:2035
> +    return hasRareData() && (rareData()->m_scrolledPosition !=
IntPoint::zero());

Extra parenthesis here.

> Source/WebCore/rendering/RenderLayer.cpp:202
> +	       // We save and restore only the scrollOffset as the other scroll

> +	       // values are recalulated.

That comment is good. I would also like to see one about us being optimistic
somewhere as the recalculated values may not match the old ones and thus
scrollOffset would be wrong (for example, would need to be clamped). Here is an
updated comment:

// We save and restore only the scrollOffset as the other scroll values are
recalulated.
// FIXME: We are assuming those scroll values did not change. If they do, our
cached scrollOffset may be wrong.