User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:69.0) Gecko/20100101 Firefox/69.0

Expected results:

During All Hands we discussed how to fix https://github.com/mozilla-mobile/android-components/issues/3182 and settled on having GeckoView report when touch events are handled. When a website is handling touch events (such as panning in Google Maps), some delegate should inform Android Components so we can disable the pull to refresh gesture.

Moving to M7 because the Fenix team would like this feature soon so they can implement pull-to-refresh: https://github.com/mozilla-mobile/android-components/issues/3182

:snorp suggested that this is something that apz can help with. Can you take a look please?

We gather this information during display list building as part of CompositorHitTestInfo (specifically, the flag CompositorHitTestFlags::eApzAwareListeners), but by the time it makes it to APZ, it has been combined with other information (when building EventRegions::mDispatchToContentRegion), so APZ can't currently tell if the page has touch event listeners specifically.

Assuming the thing we want to know is whether any element on the page has a touch event listener, it should be relatively straightforward to extend EventRegions with this information. Here's a rough outline of how that could work:

• In PaintedLayerData::AccumulateHitTestItem(), when accumulating a rect into mDispatchToContentHitRegion, if the flags contain eApzAwareListeners, set a flag mHasApzAwareListeners on the PaintedLayerData (similar to mDTCRequiresTargetConfirmation).
• In ContainerState::FinishPaintedLayerData(), propagate the mHasApzAwareListeners flag into EventRegions (again, similar to mDTCRequiresTargetConfirmation).
• In APZCTreeManager::UpdateHitTestingTree(), as we traverse the scroll nodes and propagate their event regions into the hit testing tree, accumulate a flag APZCTreeManager::mHasApzAwareListeners which tracks whether any node in the tree has mHasApzAwareListeners=true.
• When the value of APZCTreeManager::mHasApzAwareListeners changes, dispatch a notification to the relevant Android code.

Note that the above applies to non-WebRender only. With WebRender, we don't use EventRegions. We do preserve the original compositor hit test flags, including eApzAwareListeners, into the WebRender display list, but these are stored on the WebRender side in data structures which APZ normally only accesses during hit testing; it's not clear how we'd access it for this purpose.

snorp is going to talk to Jessie about this APZ issue.

Currently, we always return true from onTouchEvent() if we do anything with the event at all[0]. One nice way to solve this would be to consider returning false for events that we don't send to the DOM. We could also have a different onTouchEvent() that returned an enum so apps would know how the event was consumed. Something along these lines seems like it may be easier than pushing the touch event listener state up to the Java PanZoomController. Additionally, it means we could have correct behavior when there are touch event listeners on the page, but we're not actually hitting those regions. Botond, does that sound reasonable?

Sebastian, is something like the above basically what app developers expected with nested scrolling stuff anyway?

[0] https://searchfox.org/mozilla-central/source/widget/android/nsWindow.cpp#765

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #5)

Currently, we always return true from onTouchEvent() if we do anything with the event at all[0]. One nice way to solve this would be to consider returning false for events that we don't send to the DOM.

Note that there are a few cases to distinguish here:

1. The event is not sent to the content process at all. (This almost never happens -- only in some rare corner cases.)
2. The event is sent to the content process and may undergo some default processing in e.g. EventStateManager, but there are no JS touch event listeners registered for it.
3. The event is sent to the content process and there are touch event listeners registered for it, but they are all passive (cannot preventDefault() the event).
4. The event is sent to the content process and there are touch event listeners registered for it, some of which may preventDefault() it.

In describing the plan in comment 3, I assumed that the case we care about is distinguishing (1)-(3) from (4), since (4) is what a website would use if it does it own scrolling (and (4) is also what APZ cares about). Does this match your understanding?

Additionally, it means we could have correct behavior when there are touch event listeners on the page, but we're not actually hitting those regions.

Unfortunately, APZ doesn't currently track this information on a per-region basis.

The options here are:

1. Track a single piece of state global to a page, which is whether any element on the page has a (non-passive) touch listener. This is what the plan outlined in comment 3 does.
2. Use the per-region information that APZ currently tracks for whether it needs to wait for a content hit test before deciding whether or not to scroll something. This would catch cases where the element being touched has a (non-passive) touch listener, but it would also trigger for some other cases that don't involve touch listeners, e.g. cases where we are near the boundary of a non-rectangular element, or cases where we are over an inactive scroll frame. If GeckoView doesn't mind some amount of false positive "there is a touch listener" outcomes, this is probably the easiest to implement.
3. Track per-region information specifically about touch listeners. This would add performance overhead because it would involve adding a new region to EventRegions. I would like to avoid this if possible.

Ah, ok, I thought APZ already tracked these regions. Bummer. In that case, option #2 seems workable.

(In reply to Botond Ballo [:botond] from comment #6)

3. Track per-region information specifically about touch listeners. This would add performance overhead because it would involve adding a new region to EventRegions. I would like to avoid this if possible.

Note that with WebRender, we could implement this without performance overhead (since WebRender doesn't use EventRegions and already tracks more detailed hit-testing information).

So we could do #2 for now (non-WebRender), and once we have WebRender on Android implement #3 as an enhancement which would make the answer more accurate.

Assigning to Botond because snorp says he's looking at this bug now.

This bug is still P1 because Fenix's dynamic bottom nav bar depends on this bug.

(In reply to Botond Ballo [:botond] from comment #8)

(In reply to Botond Ballo [:botond] from comment #6)

3. Track per-region information specifically about touch listeners. This would add performance overhead because it would involve adding a new region to EventRegions. I would like to avoid this if possible.

Note that with WebRender, we could implement this without performance overhead (since WebRender doesn't use EventRegions and already tracks more detailed hit-testing information).

So we could do #2 for now (non-WebRender), and once we have WebRender on Android implement #3 as an enhancement which would make the answer more accurate.

That sounds great.

Adding this bug to GV's September sprint.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #5)

Currently, we always return true from onTouchEvent() if we do anything with the event at all[0]. One nice way to solve this would be to consider returning false for events that we don't send to the DOM. We could also have a different onTouchEvent() that returned an enum so apps would know how the event was consumed.

I did some code reading to page in the interpretation of the nsEventStatus return value of APZInputBridge::ReceiveInputEvent() to see if it would be suitable for the purpose of communicating whether an event needs to be dispatched to content before handling in APZ, and I think it's not; we currently return nsEventStatus_eConsumeDoDefault in most cases whether or not the event needs to be dispatched to content.

Instead, we can modify ReceiveInputEvent() to provide a "dispatch to content" flag as an additional result.

This is currently only populated for touch events.

Depends on D46376

Depends on D46377

The patches here implement both approach #2 from comment 6, and the WebRender enhancement described in comment 8.

So, for WebRender, we provide an accurate answer, and for non-WebRender we will have some false positives.

https://hg.mozilla.org/mozilla-central/rev/ffc0e1cd5297
https://hg.mozilla.org/mozilla-central/rev/71bd58032695
https://hg.mozilla.org/mozilla-central/rev/bb47781a80b9

firefox70=wontfix because we don't need to uplift this fix to GeckoView Beta.

Clearing needinfo for Sebastian because this bug has since been fixed.

I'm currently working on the pull to refresh implementation in A-C. When is the onTouchEventForResult method called? I haven't been able to trigger it when debugging.

(In reply to Tiger Oakes from comment #20)

I'm currently working on the pull to refresh implementation in A-C. When is the onTouchEventForResult method called? I haven't been able to trigger it when debugging.

James added the onTouchEventForResult API in bug 1557411. I needinfo'd him in that bug.