Fenix: [Meta] Pull to refresh issues

[Mugurell]

After enabling "pull to refresh" in https://github.com/mozilla-mobile/fenix/issues/3262 a number of issues were discovered.

Most of the issues relate to the following:

• [x] No differentiation between inner / outer level scrolls.
  We should only show the throbber for top level scrolls.
  GV ticket - https://bugzilla.mozilla.org/show_bug.cgi?id=1627737

• [x] For websites with their own touch handlers but which do not consume the scroll we should allow pull to refresh.
  GV ticket - https://bugzilla.mozilla.org/show_bug.cgi?id=1633322
  Other browsers like Fennec (based on the behavior of the AwesomeBar / overscroll effect) or Chrome correctly support this.
  This needs a deeper investigation and maybe a complete rethink of how pull to refresh / dynamic toolbar works.

• [x] For websites with their own touch handlers we might show a glimpse of the throbbler.
  GV - ticket - https://bugzilla.mozilla.org/show_bug.cgi?id=1631754

• [x] No actual differentiation between zooming / scrolling gestures - https://github.com/mozilla-mobile/fenix/issues/10241

• [x] "Cancel" on a repost prompt doesn't hide the throbber - https://github.com/mozilla-mobile/fenix/issues/9799

• [x] Allow users to disable gesture based features - https://github.com/mozilla-mobile/fenix/issues/10240

• [ ] Issues with google maps' bottom sheet:

  • [x] the wrong elements scroll - fenix tracking issue | GV ticket
  • [ ] rendering issues - GV ticket
  • [ ] scroll issues - GV ticket
     

• [ ] Support more CSS and Event properties related to pull to refresh - https://bugzilla.mozilla.org/show_bug.cgi?id=1679316

• [ ] Avoid unintended refreshes - https://github.com/mozilla-mobile/fenix/issues/16577

 
~As such we will disable pull to refresh until all of the above issues are resolved.~
~- [x] https://github.com/mozilla-mobile/fenix/issues/9770~

 
With most of the issues fixed this is ready for nightly but should not get into Beta / Release until all issues are fixed.

• [x] Ticket to enable feature in Nightly/Debug - https://github.com/mozilla-mobile/fenix/pull/16060

[abodea]

Hi, below you can find all the related bugs to the Pull to refresh feature.
| Website | Bug ID | Reproducible? | Comments |
| :--: | :--: | :--: | :--: |
| https://bing.com/covid | 9724 | ✔️ | |
| Phonearena.com | 9720 | ✔️ | |
| twitter | 9718 | ✔️ | |
| GitHub | 9708 | ✔️ | |
| uBlock Origin settings | 9715 | ✔️ | |
| Bugzilla | 9715 | ✔️ | Bugzilla bug 1415628|

[caugner]

Please also consider disabling pull to refresh after the user has started to fill out a form:

1. Open https://www.caresteouvert.fr/@48.861687,2.333530,17.81/place/n4246099576
2. Select "Report a change"
3. "Select "No"
4. Type in some text
5. (Accidentally) pull down
6. Page refreshes

[abodea]

Hi, @caugner thank you for the notice!
Note that I already verified the issue on the latest build you can find it here https://github.com/mozilla-mobile/fenix/issues/9770#issuecomment-610916270.
The Nightly from the Play Store will update soon and you will be able to see the change too.
I verified the issue following your steps and I couldn't reproduce it as the feature was disabled.

[caugner]

Maybe off-topic, but would be possible to link the feature to a about:config flag?

[staktrace]

See also #9799 - that should be fixed too before re-enabling, and doesn't seem to be covered by the existing issues on this ticket.

[caugner]

The Nightly from the Play Store will update soon and you will be able to see the change too.

My Nightly still pulls to refresh, despite being on the latest Play Store version. This is really frustrating, because I keep losing form input (e.g. on Twitter and in Mattermost).

[cadeyrn]

Play Store updates are paused due to https://bugzilla.mozilla.org/show_bug.cgi?id=1628413.

[liuche]

@Mugurell it looks like the GV bug has been fixed, so it should be in Fenix Nightly. I can't tell if the other issue is a GV bug, but if all the fixes are ready, is this something your team could prioritize in a sprint? It will be important for PWAs to be able to refresh #204 .

[TitanNano]

@liuche PWAs can currently be refreshed via the site control notification.

[3j9fkyahunqoxwqu]

Sorry for off-topic but please consider making pull to refresh optional.

That is really irritating for some people with accessibility problems like me.

Thank you for all your hard working.

Stay safe.
Regards.

[abodea]

Thank you for this suggestion @3j9fkyahunqoxwqu, note that I will add a feature request regarding this.

[sblatz]

@Mugurell looks like the GV fix is about to land. Once it does, is there anything we need to change on the AC/Fenix side or will we get these benefits for free?

[Mugurell]

@Mugurell looks like the GV fix is about to land. Once it does, is there anything we need to change on the AC/Fenix side or will we get these benefits for free?

I think we'll see an improvement based on that merge right away. Yay!

Updated now the top comment with the latest status.
The biggest issue atm, for both pull to refresh and the dynamic urlBar seems to be (based on https://bugzilla.mozilla.org/show_bug.cgi?id=1633322) that certain websites, big ones, have root element scroll listeners which will prevent us from correctly animating pull to refresh / urlBar.

[Mugurell]

Checked again the current status of this.

• https://bugzilla.mozilla.org/show_bug.cgi?id=1633322 is still blocking. But it affects the pull to refresh in the same way it currently affects the bottom dynamic toolbar. On some sites the toolbar is not animated / pull to refresh won't be available.
• #10241 is ready to work on, we know what should be done.
• #9799 needs some investigations about the best solution. Requires ac/gv.
• #10240 is ready to work on pending strings from UX.

With the above fixed I think pull to refresh would be good for nightly.

[liuche]

Adding this issue to GV High Priority list for https://bugzilla.mozilla.org/show_bug.cgi?id=1633322 (and also affects bottom dynamic toolbar)

[Mugurell]

https://bugzilla.mozilla.org/show_bug.cgi?id=1633322 is merged in AC and looks to have resolve the same underlying issue for the dynamic bottom toolbar. 🥳

[opusforlife2]

Can this not be added as a toggle in Secret Settings?

[cadeyrn]

@opusforlife2 Why do you want a secret setting? There will be an option in the customization settings, see #10240.

[opusforlife2]

Secret settings are for _unfinished_ features. This is at 3/5 related bugs fixed.

[Mugurell]

The setting won't appear if the feature is not available.
Pull to refresh is not live yet because there are still issues to be resolved.
The plan is to have the setting waiting to show up when pull to refresh will be enabled.

[opusforlife2]

Isn't that what secret settings are for? The first paint feature is behind a toggle, and it still has bugs to be resolved.

[cadeyrn]

It's not the same. There was a need for a setting for the "first paint feature" because user feedback was needed. For the "pull to refresh feature" the issues seems to be well known. If you really like to try it out now you only have to change the feature flag to true in the code and press the green button in Android Studio.

[foscoj]

Very nice feature, love it!
Sometimes there is an unintended refresh when I try pinch to zoom, but it is working great so far!

[Felk]

I am trying to build a website with a canvas to draw on. This requires me to prevent the pull-to-refresh. I took a look at this stackoverflow post and tried the following, simultaneously:

• give the canvas the CSS directive touch-action: none;
• call preventDefault() on the touchmove event
• give the canvas and the body the CSS directive overscroll-behavior: contain;

Firefox nightly still performed the pull-to-refresh gesture. On Chrome adding the touch-action: none; directive to the canvas was sufficient to prevent the pull-to-refresh gesture.

[masayuki-nakano]

* call `preventDefault()` on the touchmove event

Firefox nightly still performed the pull-to-refresh gesture.

As a module owner of DOM Events, I can say this is obviously a bug of Fenix because touchmove must be cancelable event.
https://w3c.github.io/touch-events/#list-of-touchevent-types

[masayuki-nakano]

And Gecko makes touch events cancelable except touchcancel.
https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/widget/TouchEvents.h#166

[mstange]

I've filed #16574, https://github.com/mozilla-mobile/android-components/issues/9043 and #16576 on the webcompat issues enumerated in the previous comments. @Mugurell, could you please add those to the list?
I've also filed #16577 which is not strictly a webcompat issue but should help reduce accidental triggers.

[emilio]

I filed https://github.com/mozilla-mobile/fenix/issues/16574 / https://github.com/mozilla-mobile/fenix/issues/16598 on other compat issues I found.