Open Bug 1795420 Opened 3 years ago Updated 2 years ago

GeckoView - InputResultDetail syncronization issues

Categories

(Firefox for Android :: Browser Engine, defect)

All
Android
defect

Tracking

()

People

(Reporter: csadilek, Unassigned)

Details

From github: https://github.com/mozilla-mobile/android-components/issues/10585.

Seen on https://github.com/mozilla-mobile/fenix/issues/18451#issuecomment-874234291 - https://sortablejs.github.io/Sortable/

Having pull to refresh enabled will not allow a second reorder down of the items.

https://user-images.githubusercontent.com/11428869/125321803-bbec3480-e345-11eb-8940-92df236e4c6b.mp4

This is happening because with a log here in that particular scenario (that can be repeated for any number of times) the time from ACTION_DOWN and until we get a response from GeckoView about how that touch was handled is >100 ms (also seen ~500ms for this scenario).

This is a lot more than the usual ~50 ms
(probably there is a bug to be fixed on GeckoView's side because this happens only on the second swipe down for which GV responds that it handled the touch, not the website as we'd expect)
and because we need to be preemptive about pull to refresh, that functionality will start and in such prevent dispatching any new touch events to the child view - GeckoView. (It will actually receive ACTION_CANCEL because the parent is handling the touch).

If we are to wait until we are actually getting a response from GeckoView, like we do here https://github.com/mozilla-mobile/android-components/issues/10555 then pull to refresh won't be possible to activate for that touch.
As we are dropping MotionEvents until waiting to get the response from GeckoView we'll drop some initial ones and then when trying to start the feature (based on GeckoViews response) we'd get an Got ACTION_MOVE event but don't have an active pointer id error. (That's why we need to be preemptive about it and start the feature, send it that initial ACTION_DOWN but then cancel the feature based on the response from pull to refresh, )if that reponse comes in a reasonable time, before actually starting to show the throbber.

I think this issue (and https://github.com/mozilla-mobile/android-components/issues/10555 also) show a big caveat in our implementation and the general feature as it has to work between GeckoView and clients: The same user touch is handled both in the client and in GeckoView and then both results have to be merged but any bigger delay will break pull to refresh / dynamic toolbar or even the browser (as it's a child View in the app).

Based on this and also other features linked here I'd like to start a discussion about the opportunity of rethinking these features and reimplementing them.
The direction I propose is having both the pull to refresh and the dynamic toolbar be driven by GeckoView. Similar to the edge effect and how the dynamic toolbar was implemented in Fennec. This would ensure a consistent UX with no synchronization issues.

Asking @pocmo @agi for their opinion.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(cpeterson)
Severity: -- → S3
Flags: needinfo?(cpeterson)
You need to log in before you can comment on or make changes to this bug.