New issue
Advanced search Search tips

Issue 602476 link

Starred by 2 users
Status: Fixed
Owner:
bokan@chromium.org
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Overscroll affected by page scale when scrolling on main thread

Project Member Reported by bokan@chromium.org, Apr 11 2016 
Version: 52.0.2705.0
OS: Android

What steps will reproduce the problem?
(1) Open any scrollable page
(2) Zoom way in
(3) Scroll all the way to the bottom. Try scrolling more to activate the overscroll glow.

What is the expected output?
The overscroll glow should appear as usual

What do you see instead?
The glow appears more slowly the further you zoom in.

The problem is in EventHandler, when we pass the gestureEvent location into handleOverscroll. The eventual handler of the overscroll message expects it to be in viewport coordinates but gestureEvent.position is in root frame coordinates.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 13 2016 

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

commit e53a10f9b029bf4367567a170fc77d6d6d42fe91
Author: bokan <bokan@chromium.org>
Date: Wed Apr 13 23:48:31 2016

Convert main-thread overscroll parameters into viewport space.

The overscroll functions in the Browser expect these values to be in the
viewport coordinate space. i.e. If the page is scaled by a factor of 2,
if the user moves their finger by 10px and all of it is consumed by overscroll,
the overscroll method should get back a delta of 10px, even though we tried
scrolling the root frame by 5px.

More generally, coordinates coming out of Blink should almost always be in
viewport/renderer/widget coordinates since it's most useful in the renderer and
it's difficult for client code to compensate for the visual viewport transform
since that's an internal implementation detail of Blink (mostly).

Also cleaned up the naming of these parameters through the call chain.

BUG= 602476 

Review URL: https://codereview.chromium.org/1879103002

Cr-Commit-Position: refs/heads/master@{#387154}

[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/content/renderer/input/render_widget_input_handler.h
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/content/renderer/render_widget.cc
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/content/renderer/render_widget.h
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/e53a10f9b029bf4367567a170fc77d6d6d42fe91/third_party/WebKit/public/web/WebWidgetClient.h

Comment 2 by bokan@chromium.org, Apr 19 2016

Status: Fixed (was: Started)
►

Sign in to add a comment