Make window.scroll properties relative to the layout viewport.

Make window.scroll properties relative to the layout viewport.

window.scroll properties being relative to visual viewport breaks some pages
under pinch-zoom. The original plan was to have all APIs reflect the layout
viewport (and introduce new APIs for visual). This CL is an initial step
towards that and the window.scroll properties will be relative to the layout
viewport if the scrollLayoutViewport setting is used.

Future CL will actually use this flag to test the implications of this change.

BUG=489206

[ymalik, 2015-09-21 14:48:21]

ymalik@chromium.org changed reviewers:
+ bokan@chromium.org

[ymalik (do not use), 2015-09-21 15:06:17]

ymalik@google.com changed reviewers:
+ ymalik@google.com

[bokan, 2015-09-21 16:49:12]

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
File
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html
(right):

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:47:
testFailed('This test requires window.internals');
should you return after this?

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:57:
var visualInnerHeight = window.innerHeight;
Please also add a case for verifying the innerWidth/Height.

PS, your test cases are really nice, but for tests like this where you only have
a small number of things to test its fine to just have a single function set and
test a bunch of settings - it's quicker to write and easy to read. OTOH, this is
totally fine too if you don't mind putting in the effort :)

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:69:
{x: 0, y: 2000, expectedX: 0, expectedY:maxScrollHeightScrollVisual,
scrollLayoutViewport:false},
Please also test the horizontal axis

https://codereview.chromium.org/1356113002/diff/1/Source/core/frame/Settings.in
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/1356113002/diff/1/Source/core/frame/Settings....
Source/core/frame/Settings.in:243: scrollLayoutViewport initial=false
The name is somewhat misleading. I would call it something like
"inertVisualViewport" or "invisibleVisualViewport".

[ymalik (do not use), 2015-09-21 20:35:50]

Worked on review comments.

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
File
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html
(right):

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:47:
testFailed('This test requires window.internals');
On 2015/09/21 16:49:12, bokan wrote:
> should you return after this?

Absolutely!

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:57:
var visualInnerHeight = window.innerHeight;
On 2015/09/21 16:49:12, bokan wrote:
> Please also add a case for verifying the innerWidth/Height.
> 
> PS, your test cases are really nice, but for tests like this where you only
have
> a small number of things to test its fine to just have a single function set
and
> test a bunch of settings - it's quicker to write and easy to read. OTOH, this
is
> totally fine too if you don't mind putting in the effort :)

Acknowledged.

That's fair. I was trying to follow the same format as other tests in this dir.

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:69:
{x: 0, y: 2000, expectedX: 0, expectedY:maxScrollHeightScrollVisual,
scrollLayoutViewport:false},
On 2015/09/21 16:49:12, bokan wrote:
> Please also test the horizontal axis

Done.

https://codereview.chromium.org/1356113002/diff/1/Source/core/frame/Settings.in
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/1356113002/diff/1/Source/core/frame/Settings....
Source/core/frame/Settings.in:243: scrollLayoutViewport initial=false
On 2015/09/21 16:49:12, bokan wrote:
> The name is somewhat misleading. I would call it something like
> "inertVisualViewport" or "invisibleVisualViewport".

Done. Thanks, I was struggling with the name for the setting!

[bokan, 2015-09-21 20:59:56]

bokan@chromium.org changed reviewers:
+ rbyers@chromium.org

[bokan, 2015-09-21 20:59:57]

lgtm. +Rick for public/web OWNER

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
File
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html
(right):

https://codereview.chromium.org/1356113002/diff/1/LayoutTests/fast/scroll-beh...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:57:
var visualInnerHeight = window.innerHeight;
On 2015/09/21 20:35:49, ymalik wrote:
> On 2015/09/21 16:49:12, bokan wrote:
> > Please also add a case for verifying the innerWidth/Height.
> > 
> > PS, your test cases are really nice, but for tests like this where you only
> have
> > a small number of things to test its fine to just have a single function set
> and
> > test a bunch of settings - it's quicker to write and easy to read. OTOH,
this
> is
> > totally fine too if you don't mind putting in the effort :)
> 
> Acknowledged.
> 
> That's fair. I was trying to follow the same format as other tests in this
dir.

Good, consistency is usually the trump card.

https://codereview.chromium.org/1356113002/diff/20001/LayoutTests/fast/scroll...
File
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html
(right):

https://codereview.chromium.org/1356113002/diff/20001/LayoutTests/fast/scroll...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:8:
overflow: hidden;
I typically use CSS to give the scrollbar no width/height without affecting
scrollability, see keyboard-scroll-page-scale.html

If this works in this case its fine too though

[ymalik (do not use), 2015-09-23 20:19:17]

@rbyers gentle ping :)

[Rick Byers, 2015-09-24 00:00:24]

sorry I missed the first mention

public/ LGTM

[ymalik, 2015-09-24 00:17:42]

https://codereview.chromium.org/1356113002/diff/20001/LayoutTests/fast/scroll...
File
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html
(right):

https://codereview.chromium.org/1356113002/diff/20001/LayoutTests/fast/scroll...
LayoutTests/fast/scroll-behavior/main-frame-pinch-scrolls-layout-viewport.html:8:
overflow: hidden;
On 2015/09/21 20:59:57, bokan wrote:
> I typically use CSS to give the scrollbar no width/height without affecting
> scrollability, see keyboard-scroll-page-scale.html
> 
> If this works in this case its fine too though

Acknowledged.

[ymalik, 2015-09-24 14:43:11]

ymalik@chromium.org changed reviewers:
- bokan@chromium.org, rbyers@chromium.org, ymalik@google.com