New issue
Advanced search Search tips

Issue 806355 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] Rotation anchoring broken on Android

Project Member Reported by bokan@chromium.org, Jan 26 2018

Issue description

Chrome Version       : 66.0.3332.0
OS Version: Android

What steps will reproduce the problem?
1. Turn on --root-layer-scrolls
2. Visit any scrollable page, scroll down
3. Rotate device

What is the expected result?
The page rotates and the scroll offset is preserved.

What happens instead of that?
The page jumps back up to the top.


Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3322.3 Safari/537.36



 

Comment 1 by bokan@chromium.org, Jan 26 2018

Blocking: 417782

Comment 2 by bokan@chromium.org, Jan 30 2018

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1f559672cf308289c52afc8f114a9057443e602

commit f1f559672cf308289c52afc8f114a9057443e602
Author: David Bokan <bokan@chromium.org>
Date: Wed Jan 31 18:18:33 2018

Fix rotation anchoring with root-layer-scrolls

This CL fixes coordinate differences in RotationViewportAnchor that were
assuming pre-RLS coordinates. This CL fixes the conversions to use the
correct RLS-aware methods in LocalFrameView (AbsoluteToDocument, etc.)
For details, see:
https://www.chromium.org/developers/design-documents/blink-coordinate-spaces

We also avoid using Node::BoundingBox as the name is a bit of a lie
(ContainerNode::BoundingBox has specific cases for how scrollIntoView
should work with empty inlines, for example).
LayoutObject::AbsoluteBoundingBoxRect is both clearer in the coordinate
space and it makes more sense to get this directly from the LayoutObject.

Unfortunately, we were lacking test coverage so this CL also adds new
tests that would have caught these bugs.

Bug:  806355 
Change-Id: I96fea83ced6c0ce7ea8c37c4a1b82a8e9a6bb66e
Reviewed-on: https://chromium-review.googlesource.com/894887
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533332}
[modify] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/frame/RotationViewportAnchor.cpp
[modify] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/frame/RotationViewportAnchor.h
[add] https://crrev.com/f1f559672cf308289c52afc8f114a9057443e602/third_party/WebKit/Source/core/frame/RotationViewportAnchorTest.cpp

Comment 4 by bokan@chromium.org, Jan 31 2018

Status: Fixed (was: Started)

Sign in to add a comment