New issue
Advanced search Search tips

Issue 860768 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 864887

Blocking:
issue 497851



Sign in to add a comment

[css-scroll-snap] Implement scroll snapping after scrollbar clicking

Project Member Reported by sunyunjia@chromium.org, Jul 6

Issue description

After clicking on the scrollbar's track or the arrows, we should respect the snap points of the scrolled container if specified.

 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 16

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

commit e98df9c6e2269035a9abb23ca8fd8dc906ad3d38
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Mon Jul 16 01:04:21 2018

Snaps after scrollbar clicking.

After user clicking on the scrollbar's track or arrows, we should snap
to the closest snap position of the container. This is done by simply
moving SnapAfterScrollbarDragging() from
ScrollableArea::MouseReleasedScrollbar() up to Scrollbar::MouseUp().

This patch also rewrites the previous
snaps-after-scrollbar-dragging.html test using the gesture-util.js, and
adds the new clicking tests to it as well.

Bug:  860768 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I75267c3fc0917ecf5c14673a1eb480a047d03824
Reviewed-on: https://chromium-review.googlesource.com/1127153
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575191}
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[delete] https://crrev.com/73f3b43c1a65d881bc4b5a60608f3019f635dfb7/third_party/WebKit/LayoutTests/fast/scroll-behavior/snaps-after-scrollbar-dragging.html
[add] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/WebKit/LayoutTests/resources/gesture-util.js
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/platform/scroll/scrollable_area.cc
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/platform/scroll/scrollable_area.h
[modify] https://crrev.com/e98df9c6e2269035a9abb23ca8fd8dc906ad3d38/third_party/blink/renderer/platform/scroll/scrollbar.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 16

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

commit dab534357763fbe122191d6c7f749816e2c08696
Author: Mikel Astiz <mastiz@chromium.org>
Date: Mon Jul 16 07:14:05 2018

Revert "Snaps after scrollbar clicking."

This reverts commit e98df9c6e2269035a9abb23ca8fd8dc906ad3d38.

Reason for revert: suspect for webkit_layout_tests consistently
failing, e.g.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%281%29/81791

Original change's description:
> Snaps after scrollbar clicking.
> 
> After user clicking on the scrollbar's track or arrows, we should snap
> to the closest snap position of the container. This is done by simply
> moving SnapAfterScrollbarDragging() from
> ScrollableArea::MouseReleasedScrollbar() up to Scrollbar::MouseUp().
> 
> This patch also rewrites the previous
> snaps-after-scrollbar-dragging.html test using the gesture-util.js, and
> adds the new clicking tests to it as well.
> 
> Bug:  860768 
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I75267c3fc0917ecf5c14673a1eb480a047d03824
> Reviewed-on: https://chromium-review.googlesource.com/1127153
> Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Reviewed-by: Majid Valipour <majidvp@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575191}

TBR=bokan@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org

Change-Id: Iab3c3c489bda0fc6b150ba0e0a5bde8de9721798
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  860768 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1138013
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575202}
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/WebKit/LayoutTests/fast/scroll-behavior/snaps-after-scrollbar-dragging.html
[delete] https://crrev.com/b480f8867d5bfa89d9698c92a710ee53b2513dc8/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/WebKit/LayoutTests/resources/gesture-util.js
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/platform/scroll/scrollable_area.cc
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/platform/scroll/scrollable_area.h
[modify] https://crrev.com/dab534357763fbe122191d6c7f749816e2c08696/third_party/blink/renderer/platform/scroll/scrollbar.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 17

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

commit 1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Tue Jul 17 22:22:59 2018

Reland "Snaps after scrollbar clicking."

This reverts commit dab534357763fbe122191d6c7f749816e2c08696.

Reason for revert: Couldn't reproduce the test failure on a local Win7
and the failure seems unrelated to my change. Try to submit it once more
and will investigate it further if it fails again.

Original change's description:
> Revert "Snaps after scrollbar clicking."
>
> This reverts commit e98df9c6e2269035a9abb23ca8fd8dc906ad3d38.
>
> Reason for revert: suspect for webkit_layout_tests consistently
> failing, e.g.
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%281%29/81791
>
> Original change's description:
> > Snaps after scrollbar clicking.
> >
> > After user clicking on the scrollbar's track or arrows, we should snap
> > to the closest snap position of the container. This is done by simply
> > moving SnapAfterScrollbarDragging() from
> > ScrollableArea::MouseReleasedScrollbar() up to Scrollbar::MouseUp().
> >
> > This patch also rewrites the previous
> > snaps-after-scrollbar-dragging.html test using the gesture-util.js, and
> > adds the new clicking tests to it as well.
> >
> > Bug:  860768 
> > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> > Change-Id: I75267c3fc0917ecf5c14673a1eb480a047d03824
> > Reviewed-on: https://chromium-review.googlesource.com/1127153
> > Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
> > Reviewed-by: David Bokan <bokan@chromium.org>
> > Reviewed-by: Majid Valipour <majidvp@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575191}
>
> TBR=bokan@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org
>
> Change-Id: Iab3c3c489bda0fc6b150ba0e0a5bde8de9721798
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  860768 
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Reviewed-on: https://chromium-review.googlesource.com/1138013
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Commit-Queue: Mikel Astiz <mastiz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575202}

TBR=bokan@chromium.org,majidvp@chromium.org,sunyunjia@chromium.org,mastiz@chromium.org

Change-Id: Iaef646348cc6e1c0a6e8b0bca3deb9296f79a7e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  860768 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1138473
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575805}
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[delete] https://crrev.com/7c8fad4aae5553262a7bb32f4c3e5a974e9f294d/third_party/WebKit/LayoutTests/fast/scroll-behavior/snaps-after-scrollbar-dragging.html
[add] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/WebKit/LayoutTests/resources/gesture-util.js
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/platform/scroll/scrollable_area.cc
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/platform/scroll/scrollable_area.h
[modify] https://crrev.com/1b3bc54c0f6abfcdff0292e66f0a9d31958ae46f/third_party/blink/renderer/platform/scroll/scrollbar.cc

Blockedon: 864887
Status: Fixed (was: Started)

Sign in to add a comment