set and get scrollTop/Left through documentElement and body should be symmetric, according to the d…

After bug 157855 is fixed, getting the scrollTop/Left properties of both documentElement and body elements follow the behavior of other Web engines (Firefox, IE, Opera12, Safari), according to the HTML mode used (strict or quirks):

- in strict mode
documentElement.scrollTop, returns the frame Y scroll position.
body.scrollTop, returns always 0.
- in quirks mode
documentElement.scrollTop, returns always 0.
body.scrollTop, returns the frame Y scroll position.

(Same logic applies for scrollLeft).

However, now Blink has an asymmetry between the way scrollTop/Left values are gotten and the way they are set, regardless of the HTML mode used:

- in both strict and quirks modes
documentElement.scrollTop = xx, is no op
body.scrollTop = xx, sets the frame Y scroll position to "xx"

Patch fixes it making the "set" operations of scrollTop/Left for documentElement and body elements symmetric to their get operations' behavior, according to the HTML mode used. It also makes Blink to match all other Web engines. Result is:

- in strict mode
documentElement.scrollTop, returns the frame Y scroll position.
documentElement.scrollTop = xx, sets the frame Y scroll position to "xx".
body.scrollTop, returns always 0.
body.scrollTop = xx, is no op.

- quirks mode
documentElement.scrollTop, returns always 0.
documentElement.scrollTop = xx, is no op.
body.scrollTop, returns the frame Y scroll position.
body.scrollTop = xx, sets the frame Y scroll position to "xx".

(Same logic applies for scrollLeft).

An optimistic approach was chosen for this behavior change, with respect to not measure the usage of these properties beforehand. Rationale is that all WebSites already have to deal with the Blink divergent behavior and other Web engines behavior anyways, so if Blink starts to follow other engines, there should not be breakages.

New tests:
* fast/dom/Element/scrollTop-scrollLeft-strict-quirks-modes.html
Updated existing tests:
* fast/events/touch/resources/compositor-touch-hit-rects-iframe.html
* fast/events/touch/resources/compositor-touch-hit-rects-iframe-fixed.html
* compositing/scissor-out-of-viewport.html
* fast/multicol/scrolling-overflow.html
* fast/css/zoom-body-scroll.html
* fast/css/resize-corner-tracking-touch.html
* fast/repaint/resources/iframe-scroll-repaint-iframe.html
* fast/events/touch/resources/compositor-touch-hit-rects-iframe-nested.html
* fast/events/touch/gesture/touch-gesture-fully-scrolled-iframe-propagates.html

BUG=303131
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159035

[Julien - ping for review, 2013-10-05 01:05:46]

https://codereview.chromium.org/25741004/diff/3001/Source/core/dom/Element.cpp
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/25741004/diff/3001/Source/core/dom/Element.cp...
Source/core/dom/Element.cpp:738:
view->setScrollPosition(IntPoint(static_cast<int>(newLeft *
frame->pageZoomFactor()), view->scrollY()));
The specification is weird and seem to imply that you shouldn't put scrollY() as
the second parameter here:

"4. If the element is the root element invoke scroll() with x as first argument
and zero as second, and, if the given value is a ScrollOptionsHorizontal object,
the given value as the third argument."

Am I missing something obvious here?

[tonikitoo_, 2013-10-07 15:03:00]

On 2013/10/05 01:05:46, Julien Chaffraix wrote:
> (...)
> "4. If the element is the root element invoke scroll() with x as first
argument
> and zero as second, and, if the given value is a ScrollOptionsHorizontal
object,
> the given value as the third argument."
> 
> Am I missing something obvious here?

Hi Julien. It is indeed weird, and I made a quick jsfiddle to verify browsers'
behavior: http://jsfiddle.net/4bRQc/4/

It turns out that Blink with the patch would match latest IE, Firefox, Opera 12
(pre-brlink) and Safari nightlies (as of Oct 7), but diverge from the WhatWG
quoted above. In summary, so scrollTop in root element would not zero the
scrollLeft. Similarly for when setting scrollLeft.

If we agree it is ok, a FIXME could be added, and followed up by a conversation
on WHATWG/bug filed.

[Julien - ping for review, 2013-10-07 16:27:25]

lgtm

> It turns out that Blink with the patch would match latest  IE, Firefox, Opera
12
> (pre-brlink) and Safari nightlies (as of Oct 7), but  diverge from the WhatWG
> quoted above. In summary, so scrollTop in root element would not zero the
> scrollLeft. Similarly for when setting scrollLeft.

> If we agree it is ok, a FIXME could be added, and followed up by a
conversation
> on WHATWG/bug filed.

Could you make sure to open a thread about it and link it from the code?

https://codereview.chromium.org/25741004/diff/3001/LayoutTests/fast/dom/Eleme...
File LayoutTests/fast/dom/Element/resources/scrollable-iframe-quirks.html
(right):

https://codereview.chromium.org/25741004/diff/3001/LayoutTests/fast/dom/Eleme...
LayoutTests/fast/dom/Element/resources/scrollable-iframe-quirks.html:13: //
Should be no-op.
Ideally this should be into its own test to avoid having a test that can now
fail for 2 reasons.

Also the comment should add more comment about why it should be a no-op.

[commit-bot: I haz the power, 2013-10-07 17:48:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a1.gomes@sisa.samsung.com/25741004/12001

[commit-bot: I haz the power, 2013-10-07 19:00:07]

Change committed as 159035