Bug 176454 – Implement CSS overscroll-behavior

• Component: CSS
• Status: RESOLVED FIXED

[Majid Valipour]

Consider implementing the new CSS scroll-boundary-behavior property.

## Specification
This feature specification is being incubated in WICG: 
https://wicg.github.io/scroll-boundary-behavior/


Note that this is first CSSWG spec which is going through this new process). 

Additional background on this feature may be found here: 
https://github.com/w3c/csswg-drafts/issues/769

## Summary
CSS scroll-boundary-behavior standardizes a supercharged version of "-ms-scroll-chaining". It
allows developers to determine the browser's behavior once a scroller has reached its full extent.
The unused delta can be propagated to the parent causing scroll chaining, create a glow/bounce
effect without chaining, or just get consumed silently. In particular when used on viewport
defining element, it controls if overscroll can be used for navigation actions such as pull-to-
refresh or swipe navigation.


scroll-boundary-behavior: auto | contain | none

  * `auto`: propagate scroll to the parent scroller. If there is no parent scroller (e.g.,
    viewport) user-agent may perform a default action (e.g. navigation) or show any appropriate
    overscroll UI affordance. This is the default value.
  * `contain`: do not propagate. The user agent may show an appropriate overscroll UI affordance
    such as bounce or glow.
  * `none`: same as contain but also prevents any overscroll UI affordance e.g. bounce or glow.

There will be scroll-boundary-behavior-{x,y} long-hands to control each axis individually.


## Current State
The specification and API has received positive feedback and support from EdgeHTML [1], Mozilla
[2], Chromium[3], and web developers [4,5]. We at Chromium have an implementation landed in
Canary (behind a feature flag**). If we don't receive any strong negative signal we plan to ship
in M63 per intent-to-ship. [3]
 

[1] https://github.com/w3c/csswg-drafts/issues/769#issuecomment-270228948
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=951793#c11
[3] https://groups.google.com/a/chromium.org/d/msg/blink-dev/OqBNF2efmFA/3ByBUKyaCgAJ
[4] https://github.com/w3c/csswg-drafts/issues/769#issuecomment-279832555
[5] https://github.com/w3c/csswg-drafts/issues/769#issuecomment-288878908

** --enable-web-platform-experimental-features or --enable-blink-features=CSSScrollBoundaryBehavior

[Radar WebKit Bug Importer]

<rdar://problem/34315997>

[Benoit Girard]

The property has been renamed to 'overscroll-behavior': https://github.com/WICG/overscroll-behavior

There's been no major changes otherwise.

[Simon Fraser (smfr)]

*** Bug 179266 has been marked as a duplicate of this bug. ***

[Frédéric Wang (:fredw)]

Created attachment 349309 [details]
Testcase

[Frédéric Wang (:fredw)]

Created attachment 349310 [details]
WIP Patch (CSS parsing only)

[Frédéric Wang (:fredw)]

Created attachment 350862 [details]
WIP Patch (CSS parsing only)

Use runtime instead of compile time flag

[Frédéric Wang (:fredw)]

Created attachment 353488 [details]
Testcase of annoying scroll chaining

This is a simple use case showing when scroll chaining can be annoying. If you scroll the overflow node to its maximum limit, release it and try to scroll it any further then the scrolling is propagated to the main frame (expected scroll chaining behavior). Then if you don't wait a bit but scroll immediately the overflow node again in the other direction, then the scrolling still applies to the main frame. The "position: fixed" is not essential here, but it can make things even more confusing since one does not see the overflow node moving when the main frame is scrolled.

[Frédéric Wang (:fredw)]

Created attachment 354920 [details]
WIP Patch (CSS parsing and preference option)

[Frédéric Wang (:fredw)]

Created attachment 355733 [details]
WIP Patch

[Frédéric Wang (:fredw)]

Created attachment 355887 [details]
WIP Patch

Just some minor change with respect to the 355733, so that "non-local boundary default actions" (e.g. scroll chaining or navigation) are now blocked for overscroll-behavior: none or contain. For now this works on macOS and GTK on attachment 349309 [details] but iOS uses a different code path.

[Frédéric Wang (:fredw)]

Created attachment 355888 [details]
WIP Patch

Sorry, I uploaded the wrong patch. Here is the correct one. Also I forgot to say that this only blocks scrolling via the mouse, not via the keyboard (my chromium release on Linux seems to have the same behavior).

[Frédéric Wang (:fredw)]

Created attachment 356006 [details]
Add overscroll behavior property to scrolling tree

[Frédéric Wang (:fredw)]

Created attachment 356410 [details]
WIP Patch

@smfr: I tried to experiment with UIScrollView's bounces/alwaysBounceHorizontal/alwaysBounceVertical but unfortunately it does not seem possible to use them to disable/enable bouncing in independent directions. I don't see any straightforward UIScrollView API to disable scroll chaining. I guess UIScrollView should probably have an overscroll property none/contain/auto so that it is possible to enable/disable things like bouncing or scrolling chaining (more generally local and non-local boundary default actions) via the UIKitSPI. I understand this is proprietary code and hence has to be handled by Apple, right? If so, can you please comment on rdar://problem/34315997 ?

This patch passes the horizontal/vertical overscroll behavior to the UI process, but probably it would be cleaner to move the OverscrollBehavior enum in ScrollTypes.h

[Frédéric Wang (:fredw)]

Created attachment 356479 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 356505 [details]
Patch (applies on top of bug 192358)

[Frédéric Wang (:fredw)]

There is a WPT manual test at https://w3c-test.org/css/css-overscroll-behavior/overscrollBehavior-manual.html

[jonjohnjohnson]

Definitely know I'm being a sort of busy body, but sincerely wondering if the webkit implementation of `overscroll-behavior` using the `contain` (or even `none`) value will allow for the same scroll containment in iOS that is showcased in in the following quite complex and hacked link:

https://bugs.webkit.org/attachment.cgi?id=356972&action=edit
https://bugs.webkit.org/attachment.cgi?id=356972

Meaning, we won't have to resort to such a mess to get this user experience?

[Ben Frain]

Really hope this is implemented soon. Any modal on iOS that has scrolling inside suffers terribly currently. Having this would mean an end to toggling overflow hidden every time we want a modal with scrollable content.

[Henrik Joreteg]

This is big deal for anybody who's implementing any sort of slide-in "hamburger" menu, etc.

For example, I'm currently working with one of the largest retailers in the world on a UI that involves a shopping cart that slides in from the right so you can quickly see cart contents without leaving your shopping experience.

As you add things to your cart that list will grow to the point where it needs to be scrolled. It's obviously not desirable to scroll the underlying page. 

What amounts to being a simple CSS rule in most browsers currently requires frustrating hacks to get it to working correctly on iOS.

Closely related... the additional requirement for adding '-webkit-overflow-scrolling: touch' is another weird one. I can't imagine a scenario where I'm wanting something to on the page to be scrollable but where I *don't* want momentum scrolling. Frankly, scrolling something without this behavior feels super broken. Seems to me it should just be the default.

Anyway, these scrolling-related issues happen increasingly as developers build more complex web pages and currently, it's one of those things that make developers curse Apple.

I think getting this resolved would buy a lot of goodwill.

Please consider implementing this. Thanks!

[Frédéric Wang (:fredw)]

Created attachment 363094 [details]
tentative rebase (not tested)

[Dmitry Semenov]

Would be great if this also allowed to prevent edge swipe to go back gesture.

This would help with horizontal carousels. Users often want to swipe carousel, but swipe to "the previous page" instead. The majority of e-commerce websites use a carousel on the product page.

At least for those that use "native" horizontal scroll with `-webkit-overflow-scrolling: touch`.

[jonjohnjohnson]

(In reply to Dmitry Semenov from comment #21)

As far as...
> This would help with horizontal carousels.
I believe spec has you covered, in that internal horizontal scrolling can be made to never chain out to the native viewport behavior with values `none`/`contain`. 

But for this general case...
> Would be great if this also allowed to prevent edge swipe to go back gesture.
Sadly, I think the current spec covers this general case only if your website already has the viewport horizontally scrolling. See https://github.com/w3c/csswg-drafts/issues/3349

- https://developers.google.com/web/updates/2017/11/overscroll-behavior
- https://drafts.csswg.org/css-overscroll-behavior-1/

[jonjohnjohnson]

(In reply to Frédéric Wang (:fredw))

Beware of https://bugs.chromium.org/p/chromium/issues/detail?id=954423 when implementing this on the viewport scroller. Considering css compat with regards to html/body propagation of this property.

[Frédéric Wang (:fredw)]

It seems there are discussions elsewhere to introduce flow-relative properties:

https://drafts.csswg.org/css-overscroll-1/#overscroll-behavior-longhands-logical

I personally think it makes sense.

[Frédéric Wang (:fredw)]

Created attachment 377722 [details]
tentative rebase (not tested)

[Frédéric Wang (:fredw)]

Created attachment 377735 [details]
tentative rebase (not tested)

[Frédéric Wang (:fredw)]

Created attachment 377789 [details]
Patch

This patch now compiles but I can't seem to make it work anymore with the testcase. This would need more debugging...

[Frédéric Wang (:fredw)]

Created attachment 378100 [details]
Patch

OK, I took a closer look into this today and this version should now again have the same behavior as my initial attempt:

* "overscroll-behavior: contain" blocks scroll chaining and go back gesture on macOS.
* "overscroll-behavior: none" blocks scroll bounces on iOS. AFAIK, we still need progress on rdar://problem/34315997 for complete support (see FIXME in ScrollingTreeScrollingNodeDelegateIOS.mm).
* no support for iframe scrolling yet (I don't remember what was the status and need to check the code, but I suspect we can have the same support as overflow: auto nodes).

[Frédéric Wang (:fredw)]

Created attachment 378196 [details]
Patch

[Paulo Sousa]

I need this css property to implement a full screen modal on my site.

In safari : when I'm on the top of my full screen modal, if i firmly scroll down, it will trigger the body scroll rubber effect not my modal scroll effect.

Here's an example: https://pzgcy.csb.app/

[Jon Lee]

rdar://problem/9925689

[cathiechen]

Created attachment 399351 [details]
rebase

Rebase the patch.

[cathiechen]

Created attachment 399352 [details]
rebase

[cathiechen]

Created attachment 399355 [details]
rebase

Rebase Fred's patch.

[Dima Voytenko]

A friendly ping here. It looks like the patches have been available for some time. Could they be reviewed?

[Simon Fraser (smfr)]

Comment on attachment 399355 [details]
rebase

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

I don't see any changes here related to scroll latching, which I'd expect are necessary to stop scroll propagation. For example, I would expect some code in ScrollingTree::handleWheelEvent/ScrollingTreeLatchingController.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3044
> +        case CSSPropertyOverscrollBehavior:
> +            return cssValuePool.createValue(std::max(style.overscrollBehaviorX(), style.overscrollBehaviorY()));
> +        case CSSPropertyOverscrollBehaviorX:
> +            return cssValuePool.createValue(style.overscrollBehaviorX());
> +        case CSSPropertyOverscrollBehaviorY:
> +            return cssValuePool.createValue(style.overscrollBehaviorY());

This code should check if the feature is enabled, right?

> Source/WebCore/css/CSSPrimitiveValueMappings.h:2209
> +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(OverscrollBehavior e)

e -> behavior

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:595
> +        return valueID == CSSValueAuto || valueID == CSSValueContain || valueID == CSSValueNone;

Does this need to check overscrollBehaviorEnabled ?

> Source/WebCore/page/EventHandler.cpp:305
> +static bool didScrollInScrollableArea(ScrollableArea& scrollableArea, double deltaX, double deltaY, unsigned deltaMode)

Can we make deltaMode be an enum first, please? It's hard to tell what it means in this context.

> Source/WebCore/page/FrameView.cpp:428
> +    if (RenderWidget* frameRenderer = frame().ownerRenderer())
> +        return frameRenderer->style().overscrollBehaviorX();

This is pretty weird; style in the parent document can affect overscroll in a subframe (any subframe, possibly cross-origin?). That doesn't seem right. Overscroll's impact should be limited to the current frame. I filed https://github.com/w3c/csswg-drafts/issues/5129

Certainly this code is wrong because the parent frame's style may not have been updated. The parent frame needs to push some state down onto this FrameView, if this is something we need to support.

[cathiechen]

Comment on attachment 399355 [details]
rebase

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

Hi Simon,
Sorry for the slow reply.
I'll upload a WIP patch later. This patch fixes the code review and supports overscroll-behavior for Mac async scroll.

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3044
>> +            return cssValuePool.createValue(style.overscrollBehaviorY());
> 
> This code should check if the feature is enabled, right?

Yes, we should check settings for this.
Added `"settings-flag": "overscrollBehavior"` to CSSProperties.json, so the propertyID of "overscrollBehavior" is invalid if the feature is disabled.
Added "overscrollBehavior" valid check test for this.

>> Source/WebCore/css/CSSPrimitiveValueMappings.h:2209
>> +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(OverscrollBehavior e)
> 
> e -> behavior

Done

>> Source/WebCore/css/parser/CSSParserFastPaths.cpp:595
>> +        return valueID == CSSValueAuto || valueID == CSSValueContain || valueID == CSSValueNone;
> 
> Does this need to check overscrollBehaviorEnabled ?

Yes, done.

>> Source/WebCore/page/EventHandler.cpp:305
>> +static bool didScrollInScrollableArea(ScrollableArea& scrollableArea, double deltaX, double deltaY, unsigned deltaMode)
> 
> Can we make deltaMode be an enum first, please? It's hard to tell what it means in this context.

OK, restored WheelEvent& and added a struct to carry ignore info here.

>> Source/WebCore/page/FrameView.cpp:428
>> +        return frameRenderer->style().overscrollBehaviorX();
> 
> This is pretty weird; style in the parent document can affect overscroll in a subframe (any subframe, possibly cross-origin?). That doesn't seem right. Overscroll's impact should be limited to the current frame. I filed https://github.com/w3c/csswg-drafts/issues/5129
> 
> Certainly this code is wrong because the parent frame's style may not have been updated. The parent frame needs to push some state down onto this FrameView, if this is something we need to support.

Yes, make sense. I think it seems reasonable to limit the impact in the current frame unless the specification clarify this.
Added FIXME in here.

[cathiechen]

Created attachment 403978 [details]
WIP-patch-based-on-Fred's-patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[cathiechen]

Created attachment 404040 [details]
WIP-patch-based-on-Fred's-patch

[Frédéric Wang (:fredw)]

Comment on attachment 404040 [details]
WIP-patch-based-on-Fred's-patch

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

I think it would be good to have more tests. Do we have more WPT tests we can use?

> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable-expected.txt:1
> +Pass test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.

invalidated

> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable.html:9
> +            document.write("Fail test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.");

invalidated

> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable.html:11
> +            document.write("Pass test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.");

I think you can simplify this: 

var result = "overscrollBehavior" in document.documentElement.style ? "PASS" : "FAIL";
document.write(`${result} test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.`);

> LayoutTests/fast/scrolling/overscroll-behavior-validate-if-enable-expected.txt:1
> +Pass test overscrollBehavior should be validate if overscrollBehaviorEnabled is enabled.

validated

> LayoutTests/fast/scrolling/overscroll-behavior-validate-if-enable.html:11
> +            document.write("Fail test overscrollBehavior should be validate if overscrollBehaviorEnabled is enabled.");

Same here.

> Source/WebCore/page/FrameView.cpp:436
> +    auto* document = frame().document();

I guess the FIXME applies here too.

[cathiechen]

Comment on attachment 404040 [details]
WIP-patch-based-on-Fred's-patch

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

Hi Fred,
Thanks for the review:)

>> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable-expected.txt:1
>> +Pass test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.
> 
> invalidated

Done

>> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable.html:9
>> +            document.write("Fail test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.");
> 
> invalidated

Done

>> LayoutTests/fast/scrolling/overscroll-behavior-invalidate-if-disable.html:11
>> +            document.write("Pass test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.");
> 
> I think you can simplify this: 
> 
> var result = "overscrollBehavior" in document.documentElement.style ? "PASS" : "FAIL";
> document.write(`${result} test overscrollBehavior should be invalidate if overscrollBehaviorEnabled is disabled.`);

Done, thanks:)

>> LayoutTests/fast/scrolling/overscroll-behavior-validate-if-enable-expected.txt:1
>> +Pass test overscrollBehavior should be validate if overscrollBehaviorEnabled is enabled.
> 
> validated

Done

>> LayoutTests/fast/scrolling/overscroll-behavior-validate-if-enable.html:11
>> +            document.write("Fail test overscrollBehavior should be validate if overscrollBehaviorEnabled is enabled.");
> 
> Same here.

Done

>> Source/WebCore/page/FrameView.cpp:436
>> +    auto* document = frame().document();
> 
> I guess the FIXME applies here too.

Done

[cathiechen]

Created attachment 404095 [details]
WIP-patch-based-on-Fred's-patch

[cathiechen]

Created attachment 404100 [details]
WIP-patch-based-on-Fred's-patch

[Ole Strøm]

Really looking forward to this one! Thanks for Fred's patch, and Cathie's further progress on it - here's to seeing it added in Safari TP/14 🤞

[Pio]

Really looking forward too, this is very important problem for PWA apps on Safari.
When can we count on solving this issue?

[Frédéric Wang (:fredw)]

Comment on attachment 404100 [details]
WIP-patch-based-on-Fred's-patch

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

> Source/WebCore/page/FrameView.cpp:425
> +    // FIXME: Currently, the impact of overscroll-behavior is limited in current frarme.

I think you mean "limited to".
Also s/frarme/frame/

> Source/WebCore/page/FrameView.cpp:436
> +    // FIXME: Currently, the impact of overscroll-behavior is limited in current frarme.

Ditto.

> Source/WebCore/page/scrolling/ScrollingTree.cpp:145
> +                if (result.wasHandled || stoppedByOverscrollBehavior) {

I see, this is the new code to address Simon's comment in https://bugs.webkit.org/show_bug.cgi?id=176454#c36. It seems the event will be considered handled as long as you have a delta in one direction and the corresponding CSS property set. But I'm not sure that's correct?

Shouldn't we have the same ignoreDeltaX and ignoreDeltaY boolean so that
- we stop to propagate if/after they become both equal to true.
- we use them to treat the corresponding wheelEvent coordinate 0 (if the ignoreDelta is true) for all the functions receiving this wheelEvent in this while loop.

> Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:242
> +        // - non-local boundary default actions (e.g. scroll chaining).

I'm curious if your change to ScrollingTree helps to implement this scroll chaining. That would mean a UIScrollView API is not needed?

> Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:243
> +        // - local boundary default actions (e.g. bouncing).

I still don't see any new thing on https://developer.apple.com/documentation/uikit/uiscrollview that would allow to disable bouncing in a single X/Y direction only. @Simon: is there any update on rdar://problem/34315997 ?

[Frédéric Wang (:fredw)]

@Simon @Cathie Sorry I've been focusing on other tasks recently, but checking this again, my understanding of the current status is:

* CSS parsing is done.

* Controlling non-local boundary default actions (e.g. scroll chaining):
  1. This patch implements it on desktop.
  2. Does Cathie's change in ScrollingTree allows to make it work on iOS too ¿? Or does it require extra uiscrollview API to handle propagation on the UI process side?
  3. IIRC Simon commented in the past Apple does not want to allow disabling the "next/previous page by swipe gesture" which I suspect the patch still does (at least that's how I initially implemented it). Maybe we shouldn't stop event propagation immediately when ignoreDeltaX && ignoreDeltaY, but instead just use these booleans to interpret the corresponding wheelEvent coordinate to 0 in relevant situations?

* Controlling local boundary default actions (e.g. bouncing)

  1. AFAIK, this does not make sense for desktop?
  2. This used to work for iOS (not sure whether that has been tested recently) but the current API on https://developer.apple.com/documentation/uikit/uiscrollview only allows to disable bouncing on one direction. It would be good to know the status of rdar://problem/34315997

* Anything else to consider here?

@Ole @Pio Thanks for the kind words and encouragement. Regarding integration Safari, the answer is unfortunately https://trac.webkit.org/wiki/FAQ#Releases. In any case we need to fix this in WebKit first. Thanks for your patience.

[Frédéric Wang (:fredw)]

(In reply to Frédéric Wang (:fredw) from comment #48)
>   2. Does Cathie's change in ScrollingTree allows to make it work on iOS too
> ¿? Or does it require extra uiscrollview API to handle propagation on the UI
> process side?

So testing it, I realized that we already never propagate scrolling to the parent scroller, unless the scroller was already at the maximum. So not sure how to interpret the spec here.

>   2. This used to work for iOS (not sure whether that has been tested
> recently) but the current API on
> https://developer.apple.com/documentation/uikit/uiscrollview only allows to
> disable bouncing on one direction. It would be good to know the status of
> rdar://problem/34315997

Just tested now and actually it works for overflow nodes, but not for subframes. Code looks correct, so I can't explain why without more debugging.

[cathiechen]

Comment on attachment 404100 [details]
WIP-patch-based-on-Fred's-patch

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

>> Source/WebCore/page/FrameView.cpp:425
>> +    // FIXME: Currently, the impact of overscroll-behavior is limited in current frarme.
> 
> I think you mean "limited to".
> Also s/frarme/frame/

Done

>> Source/WebCore/page/FrameView.cpp:436
>> +    // FIXME: Currently, the impact of overscroll-behavior is limited in current frarme.
> 
> Ditto.

Done

>> Source/WebCore/page/scrolling/ScrollingTree.cpp:145
>> +                if (result.wasHandled || stoppedByOverscrollBehavior) {
> 
> I see, this is the new code to address Simon's comment in https://bugs.webkit.org/show_bug.cgi?id=176454#c36. It seems the event will be considered handled as long as you have a delta in one direction and the corresponding CSS property set. But I'm not sure that's correct?
> 
> Shouldn't we have the same ignoreDeltaX and ignoreDeltaY boolean so that
> - we stop to propagate if/after they become both equal to true.
> - we use them to treat the corresponding wheelEvent coordinate 0 (if the ignoreDelta is true) for all the functions receiving this wheelEvent in this while loop.

Yes, this change would address Simon's comment.

And yes, indeed, we should handle them separately. It seems the wheelEvent is widely used. Maybe we can copy the wheelEvent and change the delta instead?

>> Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:242
>> +        // - non-local boundary default actions (e.g. scroll chaining).
> 
> I'm curious if your change to ScrollingTree helps to implement this scroll chaining. That would mean a UIScrollView API is not needed?

It seems not, we still need to find a way to stop scroll chaining on iOS.

[cathiechen]

Created attachment 407661 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

The current supports the desktop overscroll-behavior excepting it can't stop the swipe gesture on document scrollingElement with value "overscroll-behavior: none".

For iOS, it does not work. Still need to find a way to stop scroll chaining and the default behavior.

[Frédéric Wang (:fredw)]

Comment on attachment 407661 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

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

> Source/WebCore/page/EventHandler.cpp:376
> +            return true;

I wonder if we can just remove this early return and keep propagating the event with ignoreX/ignoreY. That way some events like "swipe gesture for previous/next" would still be taken into account (Simon said we don't want to disable that).

> Source/WebCore/page/scrolling/ScrollingTree.cpp:164
> +            }

Shouldn't we do the adjustment before calling scrollingNode.handleWheelEvent?

> Source/WebCore/page/scrolling/ScrollingTree.cpp:166
> +            if (result.wasHandled || (ignoreDeltaX && ignoreDeltaY)) {

Again, do we really need this "ignoreDeltaX && ignoreDeltaY"?

> Source/WebCore/page/scrolling/ScrollingTree.cpp:168
>                  m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);

Shouldn't you pass adjustedWheelEvent there too?

[Frédéric Wang (:fredw)]

Created attachment 407672 [details]
Testcase (setting overscroll-behavior on the iframe's html element)

I think overscroll behavior on viewport/subframes is not very well defined yet, but I suspect we should follow scroll-behavior i.e. read the property from the the documentElement.

See https://github.com/w3c/csswg-drafts/issues/5129 and https://github.com/w3c/csswg-drafts/issues/2977

[Frédéric Wang (:fredw)]

Comment on attachment 407661 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

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

> Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h:72
> +    OverscrollBehavior verticalOverscrollBehavior { OverscrollBehavior:: Auto };

The "bool operator==" member function below must also to be updated accordingly.

[cathiechen]

Comment on attachment 407661 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

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

>> Source/WebCore/page/EventHandler.cpp:376
>> +            return true;
> 
> I wonder if we can just remove this early return and keep propagating the event with ignoreX/ignoreY. That way some events like "swipe gesture for previous/next" would still be taken into account (Simon said we don't want to disable that).

Yes, looks like we can return false here, if willWheelEventStartSwipeGesture(). And it seems we need to handle the single direction scenario here. Will be updated in the new patch.

>> Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h:72
>> +    OverscrollBehavior verticalOverscrollBehavior { OverscrollBehavior:: Auto };
> 
> The "bool operator==" member function below must also to be updated accordingly.

Done, thanks!

>> Source/WebCore/page/scrolling/ScrollingTree.cpp:164
>> +            }
> 
> Shouldn't we do the adjustment before calling scrollingNode.handleWheelEvent?

It seems the adjustment should be after calling scrollingNode.handleWheelEvent, for we need to stop the scroll chaining. So the node can be scrolled, but its parent shouldn't.

>> Source/WebCore/page/scrolling/ScrollingTree.cpp:168
>>                  m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
> 
> Shouldn't you pass adjustedWheelEvent there too?

Hmm, adjustedWheelEvent's delta might be changed to zero. In `ScrollingTreeLatchingController::nodeDidHandleEvent`, it doesn't store m_latchedNodeID if `wheelEvent.delta().isZero()`. In this case, it seems right to make this node as m_latchedNodeID.

[cathiechen]

Created attachment 408125 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

Hi,

This patch adds support to overscroll-behavior of the main frame scroll latching
and single direction of overscroll-behavior.
We keep the swipe gestures. The swipe gestures will be triggered if the mainframe has scrolled to the edge.

[Frédéric Wang (:fredw)]

Comment on attachment 408125 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

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

> Source/WebCore/page/EventHandler.cpp:347
> +    bool ignoreDeltaY = false;

Can we use an IgnoreDeltaRequest instead of two bool here and maybe in other places?

> Source/WebCore/page/scrolling/ScrollLatchingController.cpp:173
> +            frameState->ignoreDelta.ignoreY = true;

These can be:

frameState->ignoreDelta.ignoreX |= ignoreDeltaFromSubFrame.ignoreX
frameState->ignoreDelta.ignoreY |= ignoreDeltaFromSubFrame.ignoreY

[cathiechen]

Created attachment 408185 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

[cathiechen]

(In reply to Frédéric Wang (:fredw) from comment #57)
> Comment on attachment 408125 [details]
> WIP-patch-supported-desktop-based-on-Fred's-patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=408125&action=review
> 
> > Source/WebCore/page/EventHandler.cpp:347
> > +    bool ignoreDeltaY = false;
> 
> Can we use an IgnoreDeltaRequest instead of two bool here and maybe in other
> places?
> 

Done, thanks!

> > Source/WebCore/page/scrolling/ScrollLatchingController.cpp:173
> > +            frameState->ignoreDelta.ignoreY = true;
> 
> These can be:
> 
> frameState->ignoreDelta.ignoreX |= ignoreDeltaFromSubFrame.ignoreX
> frameState->ignoreDelta.ignoreY |= ignoreDeltaFromSubFrame.ignoreY

Done, thanks:)

[cathiechen]

Created attachment 408212 [details]
WIP-patch-supported-desktop-based-on-Fred's-patch

[cathiechen]

Created attachment 408213 [details]
testcases of overscroll-behavior-y

[James Campbell]

Good morning I have found two bugs in this implementation.

If a container with "overflow: scroll; overscroll-behaviour: contain;" doesn't have content big enough to trigger the container to be scrollable then Safari treats the container as if overscroll-behaviour wasn't set at all as scroll stacking still happens.

If an ancestor has the "overscroll-behaviour: contain;" property then if the user tries to trigger overscroll on the child scroll container when the scroll container is already at it's scroll boundary then the elastic overscroll affordance isn't used when it should always be used.

[cathiechen]

Hi James,
Thanks for the feedback!

We now have a new patch in bug 220139 which is specific to Mac.

(In reply to James Campbell from comment #62)
> Good morning I have found two bugs in this implementation.
> 
> If a container with "overflow: scroll; overscroll-behaviour: contain;"
> doesn't have content big enough to trigger the container to be scrollable
> then Safari treats the container as if overscroll-behaviour wasn't set at
> all as scroll stacking still happens.

Thanks! Fixed in the new patch. So overscroll-behavior is effective as long as
the element's overflow is "auto" or "scroll".

> 
> If an ancestor has the "overscroll-behaviour: contain;" property then if the
> user tries to trigger overscroll on the child scroll container when the
> scroll container is already at it's scroll boundary then the elastic
> overscroll affordance isn't used when it should always be used.

Currently, the elastic overscroll affordance is triggered when scroll actually happens. If it already reaches the edge, no elastic overscroll affordance will happen, for the original procedure is scroll chaining, then mainframe perform the elastic overscroll affordance. We currently only block the scroll chaining, not trigger another elastic overscroll affordance. Yeah, I think it makes sense to add one.

Just to check; the patch will work "as long as the element's overflow is "auto" or "scroll"."

Does this include scenarios where overlflow is set like:

```
overflow-x: none;
overflow-y: auto;
```

?

[cathiechen]

(In reply to ik from comment #64)
> Just to check; the patch will work "as long as the element's overflow is
> "auto" or "scroll"."
> 
> Does this include scenarios where overlflow is set like:
> 
> ```
> overflow-x: none;
> overflow-y: auto;
> ```
> 
> ?

Yes if this element has vertical overflow.

Sorry for the slow reply...

[cathiechen]

Hi,
Sorry for the progress of overscroll-behavior is very slow.
I want to keep improving this property, but the time is limited.
I'm going to wrap up the current status and move on to other features gradually.

1. We have landed the parsing and flag patch. \o/ (https://bugs.webkit.org/show_bug.cgi?id=219305)
2. The implementing part. We have a patch ready for review. It has support overscroll-behavior
in synchronous scrolling mode and in Mac's asynchronous scrolling mode. (https://bugs.webkit.org/show_bug.cgi?id=220139)
For the asynchronous scrolling mode of the rest platforms, we wanted to support it gradually.
And on iOS, we need extra APIs of UIScrollView to provide the ability to block the scroll chaining.

Hopefully, this can bring a clear picture for you.
Still it would be great if someone can review the patch. (https://bugs.webkit.org/show_bug.cgi?id=220139)
And sorry again for the big patch.
Thanks:)

[Ole Strøm]

Thank you for the effort, Cathie!

Still looking forward for it. Many great things landing in Safari lately - great stuff!!

[Bramus]

For reference: At ±2K views per 30 days, one of the more popular posts on my blog is “Prevent overscroll/bounce in iOS MobileSafari (CSS only)” [1]. Given this popularity, it looks like developers are really interested in support for `overscroll-behavior ` (I know I am).

Above that the workaround listed in that post no longer seems to work with recent iOS versions, making this feature request even more needed.


[1] https://www.bram.us/2016/05/02/prevent-overscroll-bounce-in-ios-mobilesafari-pure-css/

I just learned that iOS Safari will get pull-to-refresh. This is clearly something a website might want to prevent from happening because it could break something. Overscroll-behavior is the obvious choice to do so.

So I'm glad it may land soon on macOS, but we really, really need this on iOS.

[Simon Fraser (smfr)]

If you have examples of sites that would be broken by pull-to-refresh, I'd be interested to hear about them.

Ok maybe broken is a bit of a stretch, but some websites would like to implement their own pull to refresh instead of reloading the entire page. For example, Twitter (the guys who pioneered PTR if I'm not mistaken) wouldn't like a page reload instead of just loading new content. 

Another scenario could be a chat dialog (popular in modern sites) where the user could cause the entire page to reload if they scroll a bit too far in the element with overflow: auto. A page reload would just be bad ux here, maybe the user loses the chat session, needs to start over explaining an issue to the support team they were chatting with.

I also wonder about standalone web apps that painstakingly try to work around the elastic scrolling effect on the body because they have an inner div that contains the actual scrollable content.

[Bruno Stasse]

If I may reinforce the point, Safari pull-to-refresh will basically be a problem for all web applications, because it might cause accidental (or encourage intentional) full-page refresh whereas it is not necessary nor desirable. These apps are designed to only fetch data when necessary, and not fully reload, otherwise the user experience is broken and way more data than needed is transferred.

This will be even more problematical for web applications implementing custom pull-to-refresh (to only fetch data), like Twitter as mentioned before, because it will most likely conflict with Safari's.

[Rado]

So sorry for the OT comment, but I have the opposite problem: scroll chaining doesn't work on mobile Safari only in one situation – vertical scroll on the text on iPhone here: https://nativecarousel.com
Any idea why and where to seek help? Thanks

[Rado]

(In reply to spacest from comment #73)
> So sorry for the OT comment, but I have the opposite problem: scroll
> chaining doesn't work on mobile Safari only in one situation – vertical
> scroll on the text on iPhone here: https://nativecarousel.com
> Any idea why and where to seek help? Thanks

Actual bug URL: https://nativecarousel.com/mobile-safari-overscroll-bug.html

[Chris Dumez]

*** Bug 226972 has been marked as a duplicate of this bug. ***

[Roman A.]

My browser application is crashed by pull-to-refresh feature. Application content was fixed by 100% width and height without any scrolls. 
Currently - slow drag gestures initiate a pull-to-refresh and content do suddenly jumps and notch. 
I need a way to disable pull-to-refresh.

[Simon Fraser (smfr)]

(In reply to Roman A. from comment #76)
> My browser application is crashed by pull-to-refresh feature. Application
> content was fixed by 100% width and height without any scrolls. 
> Currently - slow drag gestures initiate a pull-to-refresh and content do
> suddenly jumps and notch. 
> I need a way to disable pull-to-refresh.

Please file a Feedback Assistant request for that. Pull to refresh is a Safari feature, not a WebKit feature.

[Liam DeBeasi]

We would also like to see overscroll-behavior get implemented in WebKit. Pull to refresh in Safari 15 is interfering with our pull to refresh component and leaves the web app in an inconsistent state. I filed a ticket on Feedback Reporter but figured I would provide a use case for overscroll-behavior here too.

Steps to reproduce the issue in an Ionic app:

1. Open https://laughing-kirch-568bfb.netlify.app/ in Safari on iOS 15.
2. Pull down on the content and attempt to use Ionic's pull to refresh. Observe that Safari's pull to refresh kicks in and then abruptly stops.
3. Attempt to scroll up and down. Observe that Safari's pull to refresh will appear/disappear, causing layout shifts.

I would expect to be able to use overscroll-behavior to disable Safari's pull to refresh like I can in Chrome for Android, but that does not work.

Would love to see this get fully implemented. Thanks Cathie and Frederic for your work on it so far.

[marcel laverdet]

My team is working around this in our application with an effective approach which I believe to be novel, and I'd like to share:

All scrollable elements are given an extra inner content wrapper with 1px of padding on the top and bottom. When this component is rendered we manually set the scrollTop to 1px. If the user scrolls past the 1px limit and triggers the rubberbanding momentum scroll effect then the scroll gesture seems to be locked to the element for about 1 second. This locking behavior is inherent to scrollable elements in MobileSafari, and it prevents the scroll gesture from chaining to an outer element. After we detect the scrollTop changing from a negative value to 0px we can assume that the momentum effect has finished and we set the scrollTop back to 1px after a delay of 200ms, which blends in seamlessly with the momentum animation. The same logic applies in the case of scrolling past the bottom.

The implementation details are finicky and not really portable outside of our application, so I'll leave that as an exercise for the reader.

A similar method is used to prevent pull to refresh and body scrolling. The application is given a wrapper div between the body and the main content root. This element is 2px smaller in height than the applications content area, therefore the only possible values of scrollTop are: 0, 1, and 2. This wrapper div has been coined the "scroll hole". If the user manages to scroll out of the previously mentioned scrollable elements then that scroll gesture will be caught by the scroll hole. If the scroll hole is sent a scroll event it will momentarily set `overflow: hidden` on itself which cancels the scroll gesture entirely. We have to work around a lot of visualViewport bugs (which are documented here on this tracker) to get all this working correctly.

[pirijan]

+1, really encouraged by your work on this, looking forward to being able to use this

[Simon Fraser (smfr)]

This is done now, right?

[Sam Sneddon [:gsnedders]]

(In reply to Simon Fraser (smfr) from comment #82)
> This is done now, right?

https://github.com/WebKit/WebKit/blob/26f0cd5635f909d39eefc7d0ee5f893448df7f4a/Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml#L971 has this still off-by-default.

[Sam Sneddon [:gsnedders]]

Enabled by default in bug 236060.

I write my solution, I hope my method will help many people.

Css:
html{
  overscroll-behavior: none;
}
body {
    overflow-y: scroll;
}

html:
<head>
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no, height=device-height">
</head>

[giviz]

(In reply to krumplester from comment #85)
> I write my solution, I hope my method will help many people.
> 
> Css:
> html{
>   overscroll-behavior: none;
> }
> body {
>     overflow-y: scroll;
> }
> 
> html:
> <head>
> <meta name="viewport" content="width=device-width, initial-scale=1.0,
> maximum-scale=1.0, minimum-scale=1.0, user-scalable=no,
> height=device-height">
> </head>

Thank you so much my man, I've been trying to solve that for months inside my PWA and your solution worked like a charm !

You made my day ! Thanks :)