Implement scroll-snap-stop for scroll snapping

• Component: Scrolling
• Status: RESOLVED FIXED

[Majid Valipour]

This property is in scroll snap level 1 specification CR [0]. Please consider implementing this.

We (at Chromium) have implemented this and plan to ship it soon [1]. We have heard from several developers (e.g., See here [2] AirBnb, AMP [3]) that they want this. 
A particular popular usecase is to create full-size image carousels that only snap to the next image (e.g., similar to how native apps do such things) which is the motivation behind  why this feature was added to the spec in the first place. 
Normally a fling gesture snaps to the closest area near its "natural end point" which is not the right behavior in such usecases.

We have added one wpt test for this feature so far as well [4].


[0] https://www.w3.org/TR/css-scroll-snap-1/#scroll-snap-stop
[1] https://chromestatus.com/features/5439846480871424
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=823998#c3
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=920482#c1
[4] https://wpt.fyi/results/css/css-scroll-snap/scroll-snap-stop-always.html?label=master&product=chrome%5Bexperimental%5D&product=edge&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&aligned&q=scroll-snap

[Radar WebKit Bug Importer]

<rdar://problem/50708356>

[Ben Frain]

Just to add a request to implement this. We have a web-app using card like interfaces and the ability to side-swipe between them one at a time with scroll-snap would be a far better option than the default.

[Raphael Schaad]

From a developer perspective, this is a great API and works flawlessly for our use case in Chrome. There is no workaround to achieve this in Safari short of implementing custom scrolling.

[Simon Fraser (smfr)]

<rdar://problem/46111181>

[Martin Robinson]

Created attachment 419028 [details]
Patch

[Martin Robinson]

Created attachment 419030 [details]
Patch

[Martin Robinson]

Comment on attachment 419030 [details]
Patch

This still needs a bit of work.

[Martin Robinson]

Created attachment 419116 [details]
Patch

[Martin Robinson]

Comment on attachment 419116 [details]
Patch

Okay. I believe that I have sorted the failures that this patch was causing and it is now ready for review. Sorry for the noise.

[Simon Fraser (smfr)]

Comment on attachment 419116 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419116&action=review

> Source/WebCore/ChangeLog:9
> +        No new tests.

Please add some words here, with a link to the spec, and list which WPT test the feature.

> Source/WebCore/rendering/RenderLayerModelObject.cpp:204
> +    bool scrollSnapStopChanged =
> +        oldStyle && oldStyle->scrollSnapStop() != newStyle.scrollSnapStop();

Don't wrap this.

[Martin Robinson]

Created attachment 419686 [details]
Patch

[Martin Robinson]

Created attachment 419695 [details]
Patch

[Martin Robinson]

Created attachment 419707 [details]
Patch

[Martin Robinson]

(In reply to Simon Fraser (smfr) from comment #10)
> Comment on attachment 419116 [details]
> Patch

Thanks for the review!

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419116&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        No new tests.
> 
> Please add some words here, with a link to the spec, and list which WPT test
> the feature.

Sure thing. I've added the list of WPT tests here.

> 
> > Source/WebCore/rendering/RenderLayerModelObject.cpp:204
> > +    bool scrollSnapStopChanged =
> > +        oldStyle && oldStyle->scrollSnapStop() != newStyle.scrollSnapStop();
> 
> Don't wrap this.

Okay. I've also unwrapped the lines around it for consistency.

[Martin Robinson]

Comment on attachment 419707 [details]
Patch

The assertion failure on the Mac Debug WK1 bot looks to be unrelated to this change.

[EWS]

Committed r272610: <https://commits.webkit.org/r272610>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419707 [details].

[Raphael Schaad]

Created attachment 423535 [details]
The attached testcase is a basic HTML/CSS file that demonstrates minimum viable snap requirements.

[Raphael Schaad]

`scroll-snap-stop` doesn't work properly, tested in STP Release 122. Unlike in Chromium, it scrolls past `scroll-snap-stop: always` hard stops.

The previously attached testcase is a basic HTML/CSS file that demonstrates minimum viable snap requirements: outer hard snap (paging between colors), inner soft snap (aligning to black lines), un-snapped vertical scroll.

If there is a preferred way of re-opening this bug over comment here (e.g. open a new bug and reference the original bug, or comment on the metabug 218115 tracking remaining CSS scroll-snap web compatibility issues), feel free to let me know.

[Simon Fraser (smfr)]

Please open a new bug and file your test case there.

[Raphael Schaad]

Opened bug 223406