[webkit-reviews] review granted: [Bug 121876] REGRESSION (r154614): Setting the document scroll position isn't symmetric; can successfully set document.body.scrollTop, but can only read from document.documentElement.scrollTop : [Attachment 212580] Patch

Wed Sep 25 15:44:24 PDT 2013

Darin Adler <darin at apple.com> has granted gur.trio at gmail.com's request for
review:
Bug 121876: REGRESSION (r154614): Setting the document scroll position isn't
symmetric; can successfully set document.body.scrollTop, but can only read from
document.documentElement.scrollTop
https://bugs.webkit.org/show_bug.cgi?id=121876

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=212580&action=review

r=me if you address the failing test

> Source/WebCore/dom/Element.cpp:807
> +    RenderView& renderView = *document().renderView();

This is only used inside the if statement. So this should be moved inside the
if statement.

> Source/WebCore/dom/Element.cpp:811
> +	   FrameView& view = renderView.frameView();

This local variable doesn’t add anything. I suggest getting rid of it.

> Source/WebCore/dom/Element.cpp:829
> +    RenderView& renderView = *document().renderView();

This is only used inside the if statement. So this should be moved inside the
if statement.

> LayoutTests/fast/multicol/scrolling-overflow.html:1
> -<!DOCTYPE html>
>  <html>

Looks like this test is failing in the EWS bot.