w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 666 forks source link

[cssom-view] Clarify "scroll an element into view" algorithm with smooth behavior #3127

Open fred-wang opened 6 years ago

fred-wang commented 6 years ago

The recursive algorithm is described in [1]. Basically, it goes from innermost to outermost scrolling boxes, determines the target position for each of them and finally performs a scroll via [2]. Some remarks:

(A) If one of the scrolling box has smooth behavior then one must wait that the smooth scroll completes before moving to the next box or otherwise the element bounding border box returned in step 2 will be wrong. Maybe step 12 should be explicit about it.

(B) Alternatively, maybe one could run a first innermost to outermost loop to determine the final scroll positions and then perform each scroll in parallel. One advantage of this approach is that boxes with "instant" behavior are really scrolled immediately.

(C) Actually, after determining the final scroll positions one could then perform the scrolls from outermost to innermost as in (A) (i.e. at each step wait that smooth scrolls completes). As @alijuma pointed out on [3], the advantage of this approach is that it makes scroll boxes visible at each step and hence one can really see the animations of smooth scrolls.

In [3], I'm adding a new WPT test "scrollIntoView for nested scrolling nodes". It has 6 nested scrolling boxes with behaviors smooth, smooth, instant, instant, smooth and instant (from outermost to innermost). Observing the scroll positions, I obtain [4]:

Gecko 0,0,500,500,0,500 37,37,500,500,37,500 78,78,500,500,78,500 127,126,500,500,126,500 173,173,500,500,173,500 220,220,500,500,220,500 261,261,500,500,261,500 ... 499,499,500,500,499,500 500,499,500,500,499,500 500,500,500,500,500,500

Chromium 1,0,0,0,0,0 7,0,0,0,0,0 17,0,0,0,0,0 ... 488,0,0,0,0,0 496,0,0,0,0,0 499,0,0,0,0,0 500,0,0,0,0,0 500,0,0,0,0,0 500,1,0,0,0,0 500,7,0,0,0,0 500,18,0,0,0,0 500,32,0,0,0,0 500,83,0,0,0,0 ... 500,488,0,0,0,0 500,496,0,0,0,0 500,499,0,0,0,0 500,500,500,500,0,0 500,500,500,500,2,0 500,500,500,500,8,0 500,500,500,500,17,0 500,500,500,500,32,0 ... 500,500,500,500,488,0 500,500,500,500,496,0 500,500,500,500,499,0 500,500,500,500,500,500

So it looks like the spec is saying (A), Gecko is following (B) and Chromium is following (C). Hence I can't write a strict conformance test following the spec and working in all web engines. It would be good to clarify/modify the cssom-view algorithm or make clear implementers are free to use their own algorithm.

For completeness my current WIP patch for WebKit [5] is doing (A) as in the spec but it gives wrong final positions because it does not wait smooth scrolls to complete before moving to the next scrolling box.

[1] https://drafts.csswg.org/cssom-view/#scroll-an-element-into-view [2] https://drafts.csswg.org/cssom-view/#perform-a-scroll [3] https://github.com/web-platform-tests/wpt/pull/13049 [4] For the record, one can do this by adding observeScrolling(Array.from(divs), () => { var coordinates = []; divs.forEach((scrollableDiv) => { coordinates.push(scrollableDiv.scrollTop); }); console.log(coordinates.toString()); }); [5] https://bugs.webkit.org/show_bug.cgi?id=188043

jonjohnjohnson commented 6 years ago

@fred-wang https://github.com/w3c/csswg-drafts/issues/2954 seems to be tangentially related, not about the sequence of nested smooth scrollers, but still about how/when to decide the offset/distance of an element for smooth scrolling that has its in flow position changing/animating due to it's targeting. In the issues linked test case, you can remove the scroll-behavior: smooth declaration to see an "unbroken" result. Figured this was worth mentioning for the discussion that will happen here.

fred-wang commented 4 years ago

cc @cathiechen

fred-wang commented 4 years ago

Another maybe-tangential issue reported in Chromium is how to deal with multiple scrollintoview calls, for example scrollingtoview targetelement1 (smoothly) and scrollingtoview targetelement2 (instant):

<div class="smooth-scrolling-div">
      targetelement1
</div>

...

targetelement2

or any other more complex setup. See https://bugs.chromium.org/p/chromium/issues/detail?id=833617