Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(36)

Issue 2650343008: Implement Element.scrollIntoView for scroll-behavior: smooth.

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 3 weeks ago by sunyunjia
Modified:
2 hours, 7 minutes ago
Reviewers:
bokan, sof, ikilpatrick
CC:
chromium-reviews, shans, dshwang, eae+blinkwatch, kinuko+watch, rwlbuis, szager+layoutwatch_chromium.org, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews-bindings_chromium.org, jchaffraix+rendering, blink-reviews-paint_chromium.org, blink-reviews, Eric Willigers, kenneth.christiansen, rjwright, zoltan1, blink-reviews-layout_chromium.org, sof, darktears, blink-reviews-animation_chromium.org, pdr+renderingwatchlist_chromium.org, leviw+renderwatch, blink-layers+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement Element.scrollIntoView for scroll-behavior: smooth. Currently in Chrome, a call to Element.scrollIntoView will calculate the amount each scroller needs to scroll to align the Element as specified and instantly set the scroll position on that scroller (going from the innermost to the outermost scroller). If Element.scrollIntoView is called with scroll-behavior: smooth, instead of scrolling instantly, we should animate to the desired position. We can’t simply call setScrollPosition in PaintLayerScrollableArea and FrameView with ScrollBehaviorSmooth because of the following reasons: - We want to run the animation from the outermost scroller to the innermost scroller - We want to run the animation one after another. That is, the scroll animation on the child scroller should start after the animation on the parent scroller is finished. This patch adds a new class, ProgrammaticScrollCoordinator, that manages programmatic scroll animations. BUG=648446 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Patch Set 1 #

Total comments: 26

Patch Set 2 : Revised according to the comments. We are still missing tests. #

Total comments: 8

Patch Set 3 : Using isSmooth and adjust alignment. #

Patch Set 4 : Added SimTest. #

Total comments: 21

Patch Set 5 : Adds test for nested container case.:wq #

Patch Set 6 : Rename to SmoothScrollSequencer and remove the exception state. #

Patch Set 7 : Rebase #

Total comments: 30

Patch Set 8 : Abort the current animation when new scrollIntoView comes in. #

Patch Set 9 : Use writing mode to decide x and y for inline and block. Test added. #

Patch Set 10 : Added web platform test. #

Total comments: 2

Patch Set 11 : Rebase #

Total comments: 53

Patch Set 12 : Fixed the comments #

Total comments: 16

Patch Set 13 : Updated. #

Patch Set 14 : Change the default setting. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+717 lines, -56 lines) Patch
A third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +77 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/core_idl_files.gni View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +76 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.idl View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +16 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/frame/LocalDOMWindow.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +18 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/frame/RootFrameViewport.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/RootFrameViewport.cpp View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +16 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/frame/RootFrameViewportTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +17 lines, -15 lines 0 comments Download
A third_party/WebKit/Source/core/frame/ScrollIntoViewOptions.idl View 8 9 10 11 12 13 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +14 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutListBox.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutObject.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutObject.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ScrollAlignment.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ScrollAlignment.cpp View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/Page.h View 1 2 3 4 5 6 7 3 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/Page.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +16 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatSize.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +16 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollTypes.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollableArea.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/platform/scroll/SmoothScrollSequencer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +41 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/platform/scroll/SmoothScrollSequencer.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +45 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/SmoothScrollTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +288 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 98 (77 generated)
sunyunjia
Hi David, This is the patch that implements smooth scrolling for scrollIntoView. Please take a ...
3 months, 3 weeks ago (2017-01-27 18:51:57 UTC) #6
ikilpatrick
Is there an intent to implement for this?
3 months, 3 weeks ago (2017-01-28 00:46:18 UTC) #10
bokan
On 2017/01/28 00:46:18, ikilpatrick wrote: > Is there an intent to implement for this? Yah, ...
3 months, 3 weeks ago (2017-02-01 04:39:01 UTC) #11
bokan
https://codereview.chromium.org/2650343008/diff/1/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2650343008/diff/1/third_party/WebKit/Source/core/dom/Element.cpp#newcode437 third_party/WebKit/Source/core/dom/Element.cpp:437: options.setBlock("start"); according to the spec we should also initialize ...
3 months, 2 weeks ago (2017-02-02 22:51:50 UTC) #12
sunyunjia
Hi David, Here is an updated version revised according to the comments. We are still ...
3 months, 1 week ago (2017-02-10 23:25:20 UTC) #13
bokan
https://codereview.chromium.org/2650343008/diff/1/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2650343008/diff/1/third_party/WebKit/Source/core/dom/Element.cpp#newcode491 third_party/WebKit/Source/core/dom/Element.cpp:491: bounds, ScrollAlignment::alignToEdgeIfNeeded, alignment, On 2017/02/10 23:25:20, sunyunjia wrote: > ...
3 months ago (2017-02-21 21:33:01 UTC) #14
sunyunjia
For the inline and block direction, I'm having a hard time figuring out which ancestor's ...
2 months, 4 weeks ago (2017-02-24 19:11:48 UTC) #15
sunyunjia
Hi David, Could you review this updated smooth-scroll when you have time, so we could ...
1 month, 3 weeks ago (2017-03-27 19:48:32 UTC) #16
bokan
https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/core/dom/Element.cpp#newcode456 third_party/WebKit/Source/core/dom/Element.cpp:456: "Smooth ScrollIntoView is an Experimental Web" I don't think ...
1 month, 3 weeks ago (2017-03-28 16:29:54 UTC) #17
sunyunjia
https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/platform/scroll/ScrollableArea.h File third_party/WebKit/Source/platform/scroll/ScrollableArea.h (right): https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/platform/scroll/ScrollableArea.h#newcode84 third_party/WebKit/Source/platform/scroll/ScrollableArea.h:84: virtual ProgrammaticScrollCoordinator* getProgrammaticScrollCoordinator() On 2017/03/28 16:29:53, bokan wrote: > ...
1 month, 3 weeks ago (2017-03-31 15:42:45 UTC) #18
bokan
https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/platform/scroll/ScrollableArea.h File third_party/WebKit/Source/platform/scroll/ScrollableArea.h (right): https://codereview.chromium.org/2650343008/diff/60001/third_party/WebKit/Source/platform/scroll/ScrollableArea.h#newcode84 third_party/WebKit/Source/platform/scroll/ScrollableArea.h:84: virtual ProgrammaticScrollCoordinator* getProgrammaticScrollCoordinator() On 2017/03/31 15:42:45, sunyunjia wrote: > ...
1 month, 3 weeks ago (2017-03-31 20:30:47 UTC) #19
sunyunjia
Hi David, We're preparing to ship smooth scroll and I've sent out an Intent-to-ship. I've ...
1 month, 2 weeks ago (2017-04-07 13:53:21 UTC) #48
bokan
So is the plan (and resolution to the exceptions) to just ship it? I'm fine ...
1 month, 2 weeks ago (2017-04-07 15:56:53 UTC) #49
bokan
Also, in that case, I think we should wait until after the branch next week. ...
1 month, 2 weeks ago (2017-04-07 15:58:13 UTC) #50
sunyunjia
Hi David, I updated the patch and included an automated web platform test. PTAL, thanks! ...
1 week, 4 days ago (2017-05-12 18:40:25 UTC) #65
bokan
Sorry for the large amount of feedback but it's mostly about local code - I'm ...
1 week, 1 day ago (2017-05-15 17:15:28 UTC) #76
sunyunjia
Hi David, Thanks for the careful review. I learned a lot from it. I've fixed ...
4 days, 13 hours ago (2017-05-19 16:24:29 UTC) #77
bokan
> As I added a complicated nested-smooth-scroll aborts nested-smooth-scroll test, should I remove the two ...
4 days, 11 hours ago (2017-05-19 18:37:15 UTC) #83
sunyunjia
Thanks! I've updated the patch. The compiler says, static_assert failed "HeapVectorBacking doesn't support objects that ...
4 days, 7 hours ago (2017-05-19 22:30:41 UTC) #86
bokan
On 2017/05/19 22:30:41, sunyunjia wrote: > Thanks! I've updated the patch. The compiler says, static_assert ...
14 hours, 30 minutes ago (2017-05-23 15:28:12 UTC) #92
bokan
14 hours, 27 minutes ago (2017-05-23 15:31:16 UTC) #94
Oops, actually +sof@ (please see my comment above).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06