1882040 - m.youtube.com - "Pull to refresh" enabled breaks scrolling on YouTube shorts - overflow-y:hidden isn't preventing pull-to-refresh

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect, P1)

Description

[Calin Tanase]

From github: https://github.com/webcompat/web-bugs/issues/133849.

<!-- @browser: Firefox Mobile 123.0 -->
<!-- @ua_header: Mozilla/5.0 (Android 14; Mobile; rv:123.0) Gecko/123.0 Firefox/123.0 -->
<!-- @reported_with: unknown -->
<!-- @public_url: https://github.com/webcompat/web-bugs/issues/133849 -->

URL: https://m.youtube.com/shorts/

Browser / Version: Firefox Mobile 123.0
Operating System: Android 14
Tested Another Browser: Yes Edge

Problem type: Something else
Description: When trying to swipe back a short (swiping from top to bottom), the website refreshs and does not go back to the last short video.
Steps to Reproduce:
When trying to swipe back a short (swiping from top to bottom), the website refreshs and does not go back to the last short video.

<details>
<summary>Browser Configuration</summary>
<ul>
<li>None</li>
</ul>
</details>

From webcompat.com with ❤️

Change performed by the Move to Bugzilla add-on.

Comment 1

[Calin Tanase]

Attachment: yt shorts scroll.mp4

Two months ago the issue was marked as "worksforme", it seems to reproduce again with "Pull to refresh" enabled on both Firefox Release and Nightly.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 3

[eclaudiu64]

Can you take a look?

Comment 4

[Titouan Thibaud [:titouan]]

I can reproduce it too, on Firefox Nightly 126, Samsung S24. I'll link it to the Meta bug for pull-to-refresh bugs (Bug 1882722).

Comment 5

[Chris Peterson [:cpeterson]]

Titouan, did you intend to remove both the [group4] and [fxdroid] whiteboard tags from this bug?

Did you try to add or remove whiteboard tags using Bugzilla's "Change Several Bugs at Once" feature? Unfortunately, since the whiteboard field is just a free-form string and Bugzilla doesn't understand the whiteboard tags, you need to be very careful when using "Change Several Bugs at Once" to change the whiteboard field for multiple bugs. It will overwrite the bugs whiteboard fields with the new string, not add or remove individual whiteboard tags.

Comment 6

[Chris Peterson [:cpeterson]]

Setting UX Fundamentals bugs to priority P2 because we want to fix them this year.

Comment 7

[Titouan Thibaud [:titouan]]

Thanks for the heads up Chris! I don't know what happened, I haven't used the "Change Several Bugs at Once" feature and didn't intend to change the whiteboard tags. I'm putting them back.

Comment 8

[Chris Peterson [:cpeterson]]

Clearing Priority on "pull to refresh" bugs until we decide whether to disable the feature by default.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

This might be an issue under web compatibility issues. The main element of the short videos is .carousel-wrapper div element. And the element is not scrollable, no ancestor is scrollable. And the element listens touchstart/touchmove/touchend events. The touchmove listener is this;

onTouchMove: function (f) {
            e = c = f = f.touches[0].clientY;
            f = c - b;
            b = c;
            var g;
            null == (g = a.Qx) ||
            g.call(a, f)
          },

The f is an touchmove event originally, but it's replaced with `f.touches[0].clientY at the first line, so that means the event will never be preventDefault-ed. That's the reason why pull-to-refresh is triggered I think.

Comment 10

[Markus Stange [:mstange]]

The page has overflow:hidden set on the body element. It appears that Chrome does not allow pull-to-refresh on pages where the root scroller has overflow-y hidden, especially given this test:

https://source.chromium.org/chromium/chromium/src/+/main:ui/android/overscroll_refresh_unittest.cc;l=159-179;drc=90cac1911508d3d682a67c97aa62483eb712f69a

Hiro, can you take another look?

Do we currently differentiate between "page is not scrollable because it's short" and "page is not scrollable because it disallows scrolling"?
We'd still want to allow pull-to-refresh for the former.

Comment 12

[Markus Stange [:mstange]]

Chrome added the overflow-y:hidden check in February 2015, in https://codereview.chromium.org/910373002

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Thanks Markus. I actually didn't know that Chrome disallow pull-to-refresh on overflow-y:hidden root.

I would say it was kinda reasonable decision at 2015. From the commit message;

Currently, the only way to disable the pull-to-refresh effect is to
explictly preventDefault the causal touches or suppress their default
actions with touch-action: none. However, the latter prevents
any kind of accelerated scrolling, and the former can be to get right
with composed or nested scrollable content.

Use the overflow-y:hidden property on the root element as a signal to
disable the effect. This dovetails with how this property both prevents
hiding of the top controls and suppresses the overscroll glow effect.

But now all major browsers support overscroll-behavior, I would suggest using overscroll-behavior: contain (or overscroll-behavior: none) on the root element.

From the description of overscroll-behavior on MDN;

In some cases, these behaviors are not desirable. You can use overscroll-behavior to get rid of unwanted scroll chaining and the browser's Facebook/Twitter app-inspired "pull to refresh"-type behavior.

I did confirm Chrome disallow pull-to-refresh with overscroll-behavior: contain (and overscroll-behavior: none) on the root element.

Comment 14

[Markus Stange [:mstange]]

Unless you can convince Chrome to change their behavior, I do not believe it's web-compatible to ignore overflow-y: hidden.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Filed https://issues.chromium.org/issues/347060513 .

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Hiro, how easy do you think it would be to implement the overflow-y: hidden behaviour so that we can match Chrome until they changes theirs?

Comment 17

[Hiroyuki Ikezoe (:hiro)]

It should not be hard. I would say it's straight-forward, currently APZ doesn't have such kind of CSS overflow property information at all (as far as I know). So we just need to propagate it into APZ, and pretend as if overscroll-behavior: none is specified where we generate APZHandledResult.

That being said, there's only one site, m.youtube.com, we have received bug reports. Thus I'd prefer adding a web compat intervention for m.youtube.com.

Comment 18

[Botond Ballo [:botond]]

Note that propagating overflow information to APZ is something we would like to do at some point anyways, for bug 1799238.

Comment 19

[Markus Stange [:mstange]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #17)

That being said, there's only one site, m.youtube.com, we have received bug reports.

Here's another one: https://www.reddit.com/r/firefox/comments/1de7bu1/were_the_firefox_leadership_team_at_mozilla_ama/
When the page first loads, and reddit shows the modal overlay to use the app, we allow pull-to-refresh and Chrome doesn't. I was trying to make the modal disappear by swiping it down, and accidentally refreshed the page.
While the modal is visible, the body element has style="pointer-events: none; overflow: hidden;".

Comment 20

[James Graham [:jgraham]]

Unfortunately moving web compat site report bugs into other components currently breaks our ability to track broken site issues.

I'm going to move this bug back into site reports, reopen https://bugzilla.mozilla.org/show_bug.cgi?id=1902313 and try to summarize the current state of the breakage there.

Comment 21

[Botond Ballo [:botond]]

Status update on this:

• A platform fix is under discussion in bug 1902313. However, even a fix is approved, it will take time to reach the release channel.
• I've reached out to Youtube to ask if they could use html { overscroll-behavior: contain; } on the site, which would resolve the issue.

Since this is a fairly visible and high-impact issue, would it make sense to develop a web compat intervention which adds html { overscroll-behavior: contain; } in parallel with the above efforts?

Comment 22

[Hiroyuki Ikezoe (:hiro)]

FYI; In a blog post Chrome also advocates using overscroll-behavior to prevent pull-to-refresh when they introduced overscroll-behavior ; https://developer.chrome.com/blog/overscroll-behavior#disabling_pull-to-refresh .

Comment 23

[James Graham [:jgraham]]

Tom: should we ship the intervention suggested in Comment 21?

Comment 24

[Thomas Wisniewski [:twisniewski]]

Yes, I've created an intervention in https://github.com/mozilla-extensions/webcompat-addon/commit/edba1eeb6767a24a703de04f1c264d3d3bc06bab, and will attach a patch for nightly builds here shortly (no pun intended). Then we can try to uplift to the next Fenix dot-release.

Comment 25

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1882040 - Add a webcompat intervention to disable pull-to-refresh on the YouTube Shorts mobile page; r?ksenia

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a6a3cee259b
Add a webcompat intervention to disable pull-to-refresh on the YouTube Shorts mobile page; r=ksenia,webcompat-reviewers

Comment 27

[Thomas Wisniewski [:twisniewski]]

(I'm adding leave-open so when the webcompat intervention lands, the bug won't be closed, as it sounds like we want to pursue a different longer-term fix).

Comment 28

[Thomas Wisniewski [:twisniewski]]

Actually I think we can track the longer-term fix in bug 1902313, so I'll remove the leave-open keyword. (Sorry for the spam)

Comment 29

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1882040 - Add a webcompat intervention to disable pull-to-refresh on the YouTube Shorts mobile page

Comment 30

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1882040 - Add a webcompat intervention to disable pull-to-refresh on the YouTube Shorts mobile page

Comment 31

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: YouTube Shorts pages will have broken scrolling on Android due to pull-to-refresh interfering
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Visit m.youtube.com/shorts on Android, verify that pull-to-refresh test that pull-to-refresh does not happen when trying to scroll down
• Risk associated with taking this patch: Low
• Explanation of risk level: Only YouTube Shorts is affected on Android, and only to disable pull-to-refresh with the same CSS recommended by Google to disable that feature.
• String changes made/needed: No
• Is Android affected?: yes

Comment 32

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: YouTube Shorts pages will have broken scrolling on Android due to pull-to-refresh interfering
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Visit m.youtube.com/shorts on Android, verify that pull-to-refresh test that pull-to-refresh does not happen when trying to scroll down
• Risk associated with taking this patch: Low
• Explanation of risk level: Only YouTube Shorts is affected on Android, and only to disable pull-to-refresh with the same CSS recommended by Google to disable that feature.
• String changes made/needed: No
• Is Android affected?: yes

Comment 33

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/06ddbdd99693

Comment 34

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/3a6a3cee259b

Comment 35

[Botond Ballo [:botond]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #24)

Yes, I've created an intervention in https://github.com/mozilla-extensions/webcompat-addon/commit/edba1eeb6767a24a703de04f1c264d3d3bc06bab, and will attach a patch for nightly builds here shortly (no pun intended). Then we can try to uplift to the next Fenix dot-release.

Thanks very much, Thomas!

Comment 36

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/64a064d9bcfe

Comment 37

[Thomas Wisniewski [:twisniewski]]

Just a quick note for anyone doing QA on this issue; it's only meant to have an effect on Android/Fenix.

• if the patch is working, you won't be able to pull-to-refresh on https://m.youtube.com/shorts.
• you should also be able to see "YouTube Shorts" at the URL about:compat.
• you should also be able to disable the fix in about:compat, and after doing so, use pull-to-refresh on YouTube Shorts after reloading one of its tabs.

Comment 38

[Lorand Janos]

Hi,
Verified on the latest Nightly (129.0a1 from 2024-06-25), latest Beta (128.0b7) and latest RC (127.0.2) builds.
At first accessing the youtube shorts page the pull to refresh can be activated, even when the page is scrolled down first, so there can be still some unpleasant situations (I lost the ones above the one I scrolled down to, it looked like after that single refresh, it took the current one as the top one) but after that the pull-to-refresh is disabled, and it's working like I understood would be intended, I can scroll back up, without issues.
I did notice the "YouTube Shorts" item in the Interventions section of about:compat, and I can successfully enable/disable this function from there.
Since based on the instructions above the issue is only partially fixed, I'll open a new ticket about the part not fixed, and close this as verified.

Devices used:

• LG Nexus 5 (Android 6.0.1);
• Samsung Tab S8 Ultra (Android 12);
• Samsung Galaxy S23 Ultra (Android 14).

Comment 39

[Lorand Janos]

Here is the ticket created for the pull-to-refresh can be activated once issue.

Comment 40

[Jeff Muizelaar [:jrmuizel]]

FWIW, I'd rather this bug be reopened so that we retain all of the tracking.

Comment 41

[Jeff Muizelaar [:jrmuizel]]

The site patch should be deployed and working now