New issue
Advanced search Search tips

Issue 808552 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] Double tap zoom uses wrong scroll offset

Project Member Reported by bokan@chromium.org, Feb 2 2018

Issue description

Chrome Version       : 66.0.3336.4	
OS Version: Android

What steps will reproduce the problem?
1. Launch chrome with --root-layer-scrolls
2. Find a desktop page with scrolling (use "Request Desktop Site")
3. Scroll down
4. Double tap on some block

What is the expected result?
The page should scroll and zoom to center that block in view

What happens instead of that?
The page zooms to somewhere near the top of the document


 

Comment 1 by bokan@chromium.org, Feb 2 2018

Blocking: 417782
Labels: -Pri-3 M-66 Pri-1

Comment 2 by bokan@chromium.org, Feb 7 2018

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 9 2018

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

commit d437eddbe6892328354402305ee1116c9cd8f2ef
Author: David Bokan <bokan@chromium.org>
Date: Fri Feb 09 21:12:07 2018

Fix double-tap zoom with root-layer-scrolls

This requires fixing a few coordinate conversions. Prior to RLS, contents,
document and absolute coordinates are equivalent and they are not affected
by the scroll offset. When RLS is turned on, the LayoutView
PaintLayerScrollableArea (which is the Frame's "contents") is the same
size as the frame so contents coordinates are equivalent to absolute.

This CL disambiguates whether each call wants to use absolute coordinates
or document coordinates. For hit testing, we muse use absolute since that's
what hit testing code expects. The call to PixelSnappedBoundingBox also
returns a rect in absolute coordinates so I cleaned it up and fixed the
conversion to root frame.

The scroll offset/target position are expected by CC in document
coordinates so I changed those call sites.

Existing tests didn't catch this because they all perform the double tap
when the page isn't scrolled - in that case, document, absolute, frame,
content are all the same. I've added a test that performs a zoom in and
zoom out at a non-0 scroll offset.

Bug:  808552 
Change-Id: Iea512e69ac5af349ad66e866314e9f9ebb2571a7
Reviewed-on: https://chromium-review.googlesource.com/907343
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535827}
[modify] https://crrev.com/d437eddbe6892328354402305ee1116c9cd8f2ef/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/d437eddbe6892328354402305ee1116c9cd8f2ef/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
[modify] https://crrev.com/d437eddbe6892328354402305ee1116c9cd8f2ef/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/d437eddbe6892328354402305ee1116c9cd8f2ef/third_party/WebKit/Source/core/frame/LocalFrameView.h

Comment 4 by bokan@chromium.org, Feb 9 2018

Status: Fixed (was: Started)

Sign in to add a comment