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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 4 weeks ago by sunyunjia
Modified:
4 months, 1 week ago
CC:
darktears, blink-layers+watch_chromium.org, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-bindings_chromium.org, blink-reviews-dom_chromium.org, blink-reviews-layout_chromium.org, blink-reviews-paint_chromium.org, chromium-reviews, dglazkov+blink, dshwang, eae+blinkwatch, Eric Willigers, jchaffraix+rendering, kenneth.christiansen, kinuko+watch, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, rjwright, rwlbuis, shans, szager+layoutwatch_chromium.org, zoltan1
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 Review-Url: https://codereview.chromium.org/2650343008 Cr-Original-Commit-Position: refs/heads/master@{#475387} Committed: https://chromium.googlesource.com/chromium/src/+/bbf870d971d95af7ae1ee688a7ed100e3787d02b Review-Url: https://codereview.chromium.org/2650343008 Cr-Commit-Position: refs/heads/master@{#477953} Committed: https://chromium.googlesource.com/chromium/src/+/61b73ca665e42dfb7953de8a4daf1d4144805fef

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. #

Patch Set 15 : SmoothScrollSequencer should use SetOffset instead of AnimateToOffset #

Patch Set 16 : Add a short comment. #

Patch Set 17 : Rebase. #

Patch Set 18 : Rebase #

Total comments: 2

Patch Set 19 : Fixed nits. #

Patch Set 20 : Abort smooth scrolls in SetScrollOffset #

Patch Set 21 : Set default value of AbortSequencedSmoothScrolls to kAbort. #

Patch Set 22 : Get the BUILD back. #

Patch Set 23 : Rebase #

Patch Set 24 : Add ScrollType: kSequencedSmoothScroll #

Total comments: 4

Patch Set 25 : Rebase... #

Patch Set 26 : Sequenced-smooth-scrolls only happen on main thread for now. #

Total comments: 6

Patch Set 27 : Rebased. #

Patch Set 28 : Fixed nits. #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+736 lines, -61 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 13 14 15 16 17 1 chunk +77 lines, -0 lines 4 comments Download
M third_party/WebKit/Source/bindings/core/v8/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 22 23 24 25 26 1 chunk +2 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 13 14 15 16 17 18 19 20 21 22 23 24 25 26 4 chunks +77 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 1 comment Download
M third_party/WebKit/Source/core/frame/LocalFrameView.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 3 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/LocalFrameView.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 2 chunks +16 lines, -3 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 12 13 14 15 16 20 21 22 23 2 chunks +2 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 5 chunks +18 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 12 13 14 15 16 17 18 19 20 21 22 23 24 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 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 13 14 15 16 17 18 19 20 21 22 23 24 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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 14 15 16 17 18 1 chunk +3 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 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 6 chunks +25 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollTypes.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +6 lines, -2 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 4 chunks +7 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 6 chunks +25 lines, -6 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 14 15 16 17 18 19 20 21 22 23 1 chunk +47 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 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: 218 (176 generated)
sunyunjia
Hi David, This is the patch that implements smooth scrolling for scrollIntoView. Please take a ...
8 months, 4 weeks ago (2017-01-27 18:51:57 UTC) #6
ikilpatrick
Is there an intent to implement for this?
8 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, ...
8 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 ...
8 months, 3 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 ...
8 months, 2 weeks 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: > ...
8 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 ...
8 months 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 ...
6 months, 4 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 ...
6 months, 4 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: > ...
6 months, 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: > ...
6 months, 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 ...
6 months, 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 ...
6 months, 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. ...
6 months, 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! ...
5 months, 1 week 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 ...
5 months, 1 week 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 ...
5 months 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 ...
5 months 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 ...
5 months 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 ...
5 months ago (2017-05-23 15:28:12 UTC) #92
bokan
Oops, actually +sof@ (please see my comment above).
5 months ago (2017-05-23 15:31:16 UTC) #94
sunyunjia
On 2017/05/23 15:28:12, bokan wrote: > On 2017/05/19 22:30:41, sunyunjia wrote: > > Thanks! I've ...
5 months ago (2017-05-25 13:27:58 UTC) #105
sof
On 2017/05/25 13:27:58, sunyunjia wrote: > On 2017/05/23 15:28:12, bokan wrote: > > On 2017/05/19 ...
5 months ago (2017-05-25 14:25:11 UTC) #108
bokan
Ok, lgtm from me then :)
5 months ago (2017-05-25 14:29:33 UTC) #109
sunyunjia
Thanks bokan@ and sof@ ! jbroman@, could you please review this patch for "third_party/Webkit/Source/platform/"? Thanks!
5 months ago (2017-05-25 20:27:08 UTC) #111
jbroman
rs lgtm https://codereview.chromium.org/2650343008/diff/530001/third_party/WebKit/Source/platform/geometry/FloatSize.h File third_party/WebKit/Source/platform/geometry/FloatSize.h (right): https://codereview.chromium.org/2650343008/diff/530001/third_party/WebKit/Source/platform/geometry/FloatSize.h#newcode202 third_party/WebKit/Source/platform/geometry/FloatSize.h:202: // Allows Oilpan to create objects with ...
4 months, 3 weeks ago (2017-05-29 19:29:47 UTC) #120
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2650343008/550001
4 months, 3 weeks ago (2017-05-29 21:53:15 UTC) #127
commit-bot: I haz the power
Committed patchset #19 (id:550001) as https://chromium.googlesource.com/chromium/src/+/bbf870d971d95af7ae1ee688a7ed100e3787d02b
4 months, 3 weeks ago (2017-05-29 21:58:24 UTC) #130
tyoshino (SeeGerritForStatus)
Looks this CL made the "WebKit Linux Trusty Leak" bot red. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/5059 Unexpected Failures: * ...
4 months, 3 weeks ago (2017-05-30 02:04:19 UTC) #131
tyoshino (SeeGerritForStatus)
A revert of this CL (patchset #19 id:550001) has been created in https://codereview.chromium.org/2911103002/ by tyoshino@chromium.org. ...
4 months, 3 weeks ago (2017-05-30 02:08:49 UTC) #132
sunyunjia
@bokan, I've updated the patch by aborting SequencedSmoothScroll in more places to make sure that ...
4 months, 3 weeks ago (2017-06-01 19:21:29 UTC) #151
bokan
On 2017/06/01 19:21:29, sunyunjia wrote: > @bokan, > > I've updated the patch by aborting ...
4 months, 3 weeks ago (2017-06-02 17:21:52 UTC) #166
sunyunjia
bokan@, I added kSequencedSmoothScroll as a ScrollType to indicate whether the scroll should cancel the ...
4 months, 2 weeks ago (2017-06-05 20:04:28 UTC) #176
bokan
Thanks, I like this much better. https://codereview.chromium.org/2650343008/diff/730001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp File third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp (right): https://codereview.chromium.org/2650343008/diff/730001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp#newcode34 third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp:34: ScrollOffsetChanged(offset, kSequencedSmoothScroll); It ...
4 months, 2 weeks ago (2017-06-06 21:34:18 UTC) #183
sunyunjia
bokan@, PTAL, thanks! https://codereview.chromium.org/2650343008/diff/730001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp File third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp (right): https://codereview.chromium.org/2650343008/diff/730001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp#newcode34 third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp:34: ScrollOffsetChanged(offset, kSequencedSmoothScroll); On 2017/06/06 21:34:18, bokan ...
4 months, 2 weeks ago (2017-06-07 16:19:33 UTC) #198
bokan
Just a suggested rearrangement of one line but otherwise lgtm to reland https://codereview.chromium.org/2650343008/diff/770001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp File third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp ...
4 months, 2 weeks ago (2017-06-07 17:56:00 UTC) #199
bokan
https://codereview.chromium.org/2650343008/diff/770001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp File third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp (right): https://codereview.chromium.org/2650343008/diff/770001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp#newcode129 third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp:129: // information to the compositor thread. Oh, and please ...
4 months, 2 weeks ago (2017-06-07 17:56:53 UTC) #200
sunyunjia
https://codereview.chromium.org/2650343008/diff/770001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp File third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp (right): https://codereview.chromium.org/2650343008/diff/770001/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp#newcode34 third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp:34: if (sequenced_for_smooth_scroll_) On 2017/06/07 17:56:00, bokan wrote: > nit: ...
4 months, 2 weeks ago (2017-06-07 18:34:10 UTC) #203
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2650343008/850001
4 months, 2 weeks ago (2017-06-08 13:54:46 UTC) #212
commit-bot: I haz the power
Committed patchset #28 (id:850001) as https://chromium.googlesource.com/chromium/src/+/61b73ca665e42dfb7953de8a4daf1d4144805fef
4 months, 2 weeks ago (2017-06-08 14:00:57 UTC) #215
foolip
https://codereview.chromium.org/2650343008/diff/850001/third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html File third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html (right): https://codereview.chromium.org/2650343008/diff/850001/third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html#newcode4 third_party/WebKit/LayoutTests/external/wpt/scroll-into-view/check-scroll-position.html:4: <title> Check End Position of ScrollIntoView</title> Some feedback on ...
4 months, 1 week ago (2017-06-14 11:43:15 UTC) #217
foolip
4 months, 1 week ago (2017-06-14 11:51:47 UTC) #218
Message was sent while issue was closed.
https://codereview.chromium.org/2650343008/diff/850001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.idl (right):

https://codereview.chromium.org/2650343008/diff/850001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.idl:97: void scrollIntoView(optional
(ScrollIntoViewOptions or boolean)? arg);
This does not match the IDL in
https://drafts.csswg.org/cssom-view/#extension-to-the-element-interface and
unfortunately there can be differences in cases like this, see
https://github.com/heycam/webidl/issues/351

Can you either have the spec changed or match the spec, and make sure that both
scrollIntoView(undefined) and scrollIntoView(null) are tested?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa