1807072 - [Pull To Refresh] Avoid accidental refreshes by forcing a cooldown period after the last scroll gesture

Status: REOPENED

(GeckoView :: General, defect, P2)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/fenix/issues/16661.

Steps to reproduce

1. On a scrollable page, scroll down.
2. Scroll up to the top with a series of quick pans / flings.

Expected behavior

No pull refresh should be triggered during gestures that were intended to only scroll.

Actual behavior

As soon as the top edge of the page is hit, the next gesture instantly triggers pull to refresh.

This does not happen in Chrome. Chrome seems to enforce a cooldown period before pull to refresh is triggered. See this video for a comparison: quick-consecutive-flings.mp4

From experimenting, it feels like the cooldown period is somewhere between 300 and 500ms.

Device information

• Android device: Moto G5
• Fenix version: Firefox Nightly

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

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

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

This still reproduces on nightly 111.

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: PullToRefreshOrEdgeOverscroll.mp4

Seems to me that on Chrome the edge overflow effect and the pull to refresh throbber are excluding each other (as it should).
The same seems to happen in Fenix but just intermittently. In my testing I see that when the overascroll effect is showing the pull to refresh won't show but there are also cases in which the overscroll effect is not showing and so pull to refresh is showing, all based on the InputResultDetail api.

Seems to me that the app then behaves as expected but there is a bug in GeckoView (with the overscroll functionality being part of GeckoView[1]) / APZ that needs to be fixed there.
Raising this to Hiro, Jonathan and Botond here also since the pull to refresh polish is a current priority.

[1] https://searchfox.org/mozilla-central/rev/0907529ff72c456ddb47839f5f7ba16291f28dce/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/OverscrollEdgeEffect.java

Comment 4

[Botond Ballo [:botond]]

Comment 3 sounds like it may pertain to a different scenario (the one in bug 1807073 perhaps?), since the video does not involve a series of flings.

I looked a bit into the issue described in comment 3 anyways:

(In reply to Petru-Mugurel Lingurar [:petru] from comment #3)

Seems to me that on Chrome the edge overflow effect and the pull to refresh throbber are excluding each other (as it should).
The same seems to happen in Fenix but just intermittently. In my testing I see that when the overascroll effect is showing the pull to refresh won't show but there are also cases in which the overscroll effect is not showing and so pull to refresh is showing, all based on the InputResultDetail api.

Seems to me that the app then behaves as expected but there is a bug in GeckoView (with the overscroll functionality being part of GeckoView[1]) / APZ that needs to be fixed there.
Raising this to Hiro, Jonathan and Botond here also since the pull to refresh polish is a current priority.

At the APZ level, there is no interaction between overscroll and pull-to-refresh, for example there is no attempt to make them mutually exclusive. To the extent that they are mutually exclusive today, that's accomplished by the logic for triggering the respective effects in GeckoView (for overscroll) and Fenix (for pull-to-refresh).

I don't think that making them mutually exclusive can be implemented in APZ today, because it doesn't have enough information -- for example, it doesn't know that a particular gesture will trigger pull to refresh (and therefore it should suppress the overscroll-related messages that it sends to GeckoView), only that a subset of the conditions for triggering pull to refresh are met (with other conditions, like the pull-to-refresh feature being enabled, only known to the application layer).

(Things may work differently after the refactor discussed in https://github.com/mozilla-mobile/android-components/blob/0ae1fbfe22ff0bcb1e2f77b1f23ad39ddc0da280/docs/rfcs/0009-geckoview-dynamic-features.md, but I'm a bit hazy on the details so I can't say for sure.)

So, I think the next investigative step on this issue is in the GeckoView layer, to understand how overscroll is successfully suppressed for most gestures that trigger pull-to-refresh, and then why that suppression intermittently fails.

Comment 5

[Botond Ballo [:botond]]

As for the issue discussed in the original report / comment 0, I believe the solution approach in the bug title ("Avoid accidental refreshes by forcing a cooldown period after the last scroll gesture") remains the relevant one, and per our earlier discussion on this that will be implemented at the application layer. Please let me know if I've misunderstood.

Comment 6

[Jonathan Almeida [:jonalmeida]]

This should also be fixed with the patches from bug 1807073.

Comment 7

[Lorand Janos]

This issue is no longer reproducible on the latest Nightly build (124.0a1 from 2024-01-23).
Device used: Samsung Galaxy S23 Ultra (Android 14).
Marking the ticket as verified.

Comment 8

[Markus Stange [:mstange]]

Hmm, is this really fixed? I'm confused about how we enforce the cooldown.
This bug is asking that a new drag gesture which starts when the page is already at the end should not cause pull-to-refresh if it happens too soon after the previous scroll. Is that implemented?

Comment 9

[Jonathan Almeida [:jonalmeida]]

Thanks for catching this, Markus! We have not fixed this and is still a bug. We might have misunderstood this bug and closed it prematurely. I will re-open it for now.

Leaving Titouan's need-info on so that it can be re-triaged, as he mentioned in bug 1807071 comment 12 that a new meta would be opened for any remaining issues from the previous meta that we would want to track.

Comment 10

[Titouan Thibaud [:titouan]]

Attachment: Chrome demo

Hi,
I just tried on Chrome the steps to reproduce described on the original bug, and I see the same behavior as we have on Fenix: Pull to Refresh will be triggered by the first scroll gesture that starts when the page has reached the top, even if we are in a quick sequence of scroll gestures.

Do we still need to introduce the cooldown?

Comment 11

[Markus Stange [:mstange]]

Interesting! The original video (attachment 9189010 [details]) clearly showed that Chrome had a cooldown at some point. I think it would still be a good idea to introduce one regardless of Chrome's behavior.

I wonder why Chrome removed it. Maybe they broke it by accident. If they removed it intentionally then it would be nice to find out the reasoning.

Comment 12

[Chris Peterson [:cpeterson]]

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

Comment 13

[Polly McEldowney [:pollymce]]

I have spiked implementing a cooldown period in fenix and it is possible to do (although you don't get an overscroll edge effect during this period).
However, Chrome doesn't do this any more, as has been mentioned above. And i'm not sure whether we should proceed with this approach - the ticket seems predicated on the outdated Chrome behaviour. It may catch some instances of incorrectly interpreted scroll gestures, but this would be timing-dependent - scroll gestures performed outside the cooldown period could still be misinterpreted as pull-to-refresh. It could also be a bit annoying for people who are trying to refresh a page multiple times in quick succession, depending on how long you set the cooldown to be.
There are multiple pull-to-refresh scroll issues in the parent meta bug. Maybe we should look at those first and then decide whether we still want to implement this?

Comment 14

[Markus Stange [:mstange]]

Great, can you attach your patch here?

I feel pretty strongly that this is worth doing regardless of Chrome's behavior.

In the last week I've noticed at least four accidental page refreshes in my personal use of Fenix that would have been prevented by this.

Comment 15

[Polly McEldowney [:pollymce]]

hi Markus - I'm afraid I don't have a patch yet, just a spike.
Could you give more detail on the accidental page refreshes you've experienced please? eg. what website you were on, and what interactions took place?
I'm keen to understand the problem scenarios here, so that we can be unblocked from testing possible solutions.
We currently have a couple of open bugs around pull-to-refresh not being triggered on every gesture: 1873873 and 1878247. It could be a fine balancing act to resolve all three of these issues at the same time! So any more info you can share would be great.

Comment 16

[Markus Stange [:mstange]]

Oh, I'm not familiar with the term spike.

The way I encounter these accidental refreshes most often is as follows:

1. Read your Mastodon timeline on mastodon.social (logged in).
2. Tap on a post which is a reply on a thread, in order to view the thread.
3. Now the thread is loaded and the tapped post is in view, with the preceding posts above but scrolled offscreen.
4. Quickly scroll to the start of the thread.

Here I intuitively do two quick swipes up, but the first swipe is often enough to get me to the top and the second swipe triggers a refresh before I have time to react to (or even notice) the fact that I'm already at the top.

I agree that bug 1873873 and bug 1878247 are a reverse problem that is also worth fixing, but I believe that we can find a threshold that wouldn't make those problems noticeable worse.

Comment 17

[Timothy Nikkel (:tnikkel)]

(In reply to Markus Stange [:mstange] from comment #16)

4. Quickly scroll to the start of the thread.

Here I intuitively do two quick swipes up, but the first swipe is often enough to get me to the top and the second swipe triggers a refresh before I have time to react to (or even notice) the fact that I'm already at the top.

Yes, I do this as well, I do some quick swipes to scroll me back to the top and often one of the swipes will trigger when I'm at the top and then reload the page. It's made worse because I'm often trying to get the url bar to appear and it won't appear and the only sure fire way I've found to get it to appear is to scroll to the top.