New issue
Advanced search Search tips

Issue 788248 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] Coordinate conversion is wrong in ScrollFocusedEditableElementIntoView

Project Member Reported by bokan@chromium.org, Nov 24 2017

Issue description

I haven't come up with a repro case yet but while writing a test for ScrollFocusedEditableNodeIntoView (see https://crrev.com/c/788189) I realized VisualViewport::VisibleRectInDocument is incorrect since it relies unconditionally on the FrameView's visible rect.

ScrollFocusedEditableNodeIntoView is the only method that relies on this via the ComputeScaleAndScrollForFocusedNode method. This is used to calculate how to scroll and zoom the viewport when a small input box is focused.
 

Comment 1 by bokan@chromium.org, Nov 24 2017

Blocking: 417782

Comment 2 by bokan@chromium.org, Nov 24 2017

Status: WontFix (was: Started)
Actually, it works just fine. It's my test that's borked.

Comment 3 by bokan@chromium.org, Nov 24 2017

Status: Started (was: WontFix)
"just fine" was a bit of a stretch. There is an issue since it relies on LocalFrameView::FrameToContents which unconditionally uses the FrameView's scroll offset.

Comment 4 by bokan@chromium.org, Nov 24 2017

Summary: [root layer scrolls] Coordinate conversion methods need to account for root scrolling layer (was: [root layer scrolls] Zoom into text box animation is likely broken)
Another issue is in LocalToAbsoluteQuad which incorrectly applies the LayoutView scroll offset when RLS is turned on. This is a more general issue than I initially thought and will require plenty of changes.

Comment 5 by bokan@chromium.org, Nov 29 2017

Summary: [root layer scrolls] Coordinate conversion is wrong in ScrollFocusedEditableElementIntoView (was: [root layer scrolls] Coordinate conversion methods need to account for root scrolling layer)
Paring the scope of this bug back down.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 29 2017

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

commit 3986408908f168ff1f24625612e1205c831f897b
Author: David Bokan <bokan@chromium.org>
Date: Wed Nov 29 21:34:59 2017

[RLS] Fix ScrollFocusedEditableElementIntoView

This CL fixes coordinate conversions inside
ScrollFocusedEditableElementIntoView when root layer scrolling is
enabled. This method is responsible for scroll and zoom into view type
actions such as when tapping on an input box on a desktop page using
Chrome Android.

I've added some RLS aware methods for conversions like these in
LocalFrameView and also converted a recent fix in DataTransfer to use
them (as well as fixed a misleading naming mistake).

Bug:  788248 
Change-Id: Icd5bbf7aa86570e537c26b237f9c66f7f168b6a9
Reviewed-on: https://chromium-review.googlesource.com/795177
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520251}
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/clipboard/DataTransfer.cpp
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/clipboard/DataTransfer.h
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/3986408908f168ff1f24625612e1205c831f897b/third_party/WebKit/Source/core/frame/LocalFrameView.h

Comment 7 by bokan@chromium.org, Nov 30 2017

Status: Fixed (was: Started)

Sign in to add a comment