New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 626258 link

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



Sign in to add a comment

Scroll Anchoring fails to prevent jump from VH-unit change on hiding top controls

Project Member Reported by kenjibaheux@chromium.org, Jul 7 2016 
Version: 53.0.2780.0
OS: Android

What steps will reproduce the problem?
(1) https://s.codepen.io/burriep/debug/vGMRRo
(2) in one fling, scroll past the red block

What is the expected output?
No reflow

What do you see instead?
unmitigated reflow as the Omnibox disappears.


Root cause: the disappearance of the Omnibox "causes the red section (which is 100vh tall) to resize, causing the text below it to change position."

Comment 1 by krav...@chromium.org, Jul 7 2016

Labels: scroll-anchoring-te

Comment 2 by skobes@chromium.org, Jul 7 2016

Blocking: 558575

Comment 3 by skobes@chromium.org, Jul 7 2016

Cc: bokan@chromium.org
Summary: Scroll Anchoring fails to prevent jump from VH-unit change on hiding top controls (was: Scroll Anchoring: continuous scroll on Japanican)
This is really  issue 428132  (proposal: https://github.com/bokand/URLBarSizing), but I'm not sure why scroll anchoring doesn't compensate.

Comment 4 by skobes@chromium.org, Jul 7 2016

Blocking: -558575

Comment 5 by dtapu...@chromium.org, Jul 21 2016

Labels: Hotlist-Input-Dev
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 28 2016 

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

commit 6913f420989be6dbaed45bd7e1b60042ff119d70
Author: skobes <skobes@chromium.org>
Date: Thu Jul 28 18:36:33 2016

Fix scroll anchoring during viewport resize.

WebView::resizeWithTopControls does a synchronous layout.  Anchoring scrolls
performed during this layout were getting clobbered by ResizeViewportAnchor,
whose purpose is to scroll the viewports to preserve their combined position
(as reported by RootFrameViewport) after bounds-clamping to their new sizes.

After this change, ResizeViewportAnchor tracks the resize-induced scroll delta
while ignoring other scrolls.

BUG= 626258 ,  612305 

Review-Url: https://codereview.chromium.org/2184333002
Cr-Commit-Position: refs/heads/master@{#408439}

[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/ResizeViewportAnchor.cpp
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/ResizeViewportAnchor.h
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/RotationViewportAnchor.cpp
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/RotationViewportAnchor.h
[delete] https://crrev.com/f79d8967f4c6bc91dfb93ffd4f7972a2e39fd05e/third_party/WebKit/Source/web/ViewportAnchor.h
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/tests/VisualViewportTest.cpp
[add] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/tests/data/icb-relative-content.html
[modify] https://crrev.com/6913f420989be6dbaed45bd7e1b60042ff119d70/third_party/WebKit/Source/web/web.gypi

Comment 7 by skobes@chromium.org, Jul 28 2016 

This is half fixed (literally, we adjust for 50% of the resizes) due to the bounce suppression logic.

Comment 8 by skobes@chromium.org, Sep 6 2016

Status: Fixed (was: Available)
This is fixed at r413010.

This means scroll anchoring suppresses the jump caused by the resizing of offscreen VH-unit elements.  However it doesn't prevent the resize in the first place which is tracked by  issue 428132 .
►

Sign in to add a comment