New issue
Advanced search Search tips

Issue 913009 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 845500



Sign in to add a comment

UserScrolls with scrollbehavior: smooth shouldn't be aborted by ProgrammaticScrolls

Project Member Reported by sunyunjia@chromium.org, Dec 7

Issue description

Currently, when there are new scroll coming in, the previously queued smooth scrolls are all aborted. However, scrolls with type UserScroll should have higher priority than ProgrammaticScroll and shouldn't be aborted by ProgrammaticScrolls.

An example is smooth scroll of "Find" in Android. After user tapping "Find", the smooth ScrollIntoView of the target is queued. However, the virtual keyboard also disappears at this moment, causing the viewport to reset its offset and abort the queued smooth scrolls. In such circumstances, it is the later ProgrammaticScroll that should be aborted, not the user's smooth scroll.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 17

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

commit 6601a08a46718f26ca69dc1ca488c831986500be
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Mon Dec 17 17:29:03 2018

Smooth UserScrolls shouldn't be aborted by ProgrammaticScrolls.

Currently, when there are new scroll coming in, the previously queued
smooth scrolls are all aborted. However, scrolls with type UserScroll
should have higher priority than ProgrammaticScroll and shouldn't be
aborted by ProgrammaticScrolls.

An example is smooth scroll of "Find" in Android. After user tapping
"Find", the smooth ScrollIntoView of the target is queued. However, the
virtual keyboard also disappears at this moment, causing the viewport to
reset its offset and abort the queued smooth scrolls. In such
circumstances, it is the later ProgrammaticScroll that should be aborted,
not the user's smooth scroll.

This patch adds scroll_type_ in SmoothScrollSequencer to record the
scroll type of the current smooth scroll. FilterNewScroll() can then
decide whether to filter the incoming new scroll or abort the current
smooth scroll depending of the scroll types.

Bug:  913009 
Change-Id: I0dc7d04d79ef851aae7482d1812ad12e2a92be1d
Reviewed-on: https://chromium-review.googlesource.com/c/1374466
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617148}
[modify] https://crrev.com/6601a08a46718f26ca69dc1ca488c831986500be/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/6601a08a46718f26ca69dc1ca488c831986500be/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc
[modify] https://crrev.com/6601a08a46718f26ca69dc1ca488c831986500be/third_party/blink/renderer/core/scroll/scrollable_area.cc
[modify] https://crrev.com/6601a08a46718f26ca69dc1ca488c831986500be/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.cc
[modify] https://crrev.com/6601a08a46718f26ca69dc1ca488c831986500be/third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h

Status: Fixed (was: Started)

Sign in to add a comment