Closed Bug 1897567 Opened 6 months ago Closed 4 months ago

GeckoView should not ask APZ for an InputResult for events other than MotionEvent.ACTION_DOWN

Categories

(Fenix :: Browser Engine, task, P2)

task

Tracking

(firefox130 fixed)

RESOLVED FIXED
130 Branch
Tracking Status
firefox130 --- fixed

People

(Reporter: botond, Assigned: hiro)

References

(Blocks 3 open bugs)

Details

Attachments

(1 file)

In bug 1891094, I discovered that, since bug 1807073, GeckoView sometimes asks APZ for an InputResult for motion events of type MotionEvent.ACTION_MOVE, not just of type MotionEvent.ACTION_DOWN.

This is something that APZ does not expect, and I suspect that it may be the cause of some of the pull-to-refresh bugs blocking bug 1882722.

I'm filing this bug to track revising the GeckoView code to go back to only asking APZ for an InputResult on ACTION_DOWN events. More precisely, the revision will likely need to be made in the android-components layer, to only call the onTouchEventForDetailResult GeckoView API for ACTION_DOWN events (likely involving a modification of this call site.

Alternatively, if we find that asking for an InputResult for ACTION_MOVE events is sometimes necessary, we need to decide what the expected result is in various scenarios, and then audit the APZ code to make sure it sends the expected result.

See Also: → 1891094

My best guess here is probably we need to do the same thing what this code block does in the MotionEvent.ACTION_DOWN case.

Blocks: 1898866

D213695 is a minimum change but it fixes bug 1891094. What I am concerned is, I am not confident enough not to regress, normally our existing automated test cases makes me confident, but unfortunately in this specific case, changes in Fenix (to be precise, it's in android-components) there's no tests. I can't event add the reduced test case I attached in bug 1891094 comment 21. :/

Component: Panning and Zooming → General
Product: Core → GeckoView
Component: General → Browser Engine
Product: GeckoView → Fenix
Assignee: nobody → hikezoe.birchill
Attachment #9407399 - Attachment description: WIP: Bug 1897567 - Invoke onTouchEventForDetailResult just once for each touch block. → Bug 1897567 - Invoke onTouchEventForDetailResult just once for each touch block. r?botond!,tthibaud!
Status: NEW → ASSIGNED
Blocks: 1891094
See Also: 1891094
See Also: → 1903682
Blocks: 1852854

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

D213695 is a minimum change but it fixes bug 1891094. What I am concerned is, I am not confident enough not to regress, normally our existing automated test cases makes me confident, but unfortunately in this specific case, changes in Fenix (to be precise, it's in android-components) there's no tests. I can't event add the reduced test case I attached in bug 1891094 comment 21. :/

i would love to help progress this work if possible!
But without existing tests, it sounds very difficult to be sure that we haven't broken anything.
Do you have any ideas on how we could gain confidence that this patch works well?
eg. a list of bugs / scenarios that we could manually retest?

Thanks Polly. But the comment is outdated. I realized that some of tests exist in android-components directory and I have modified them in D213695.

Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/de69f60eb1f8 Invoke onTouchEventForDetailResult just once for each touch block. r=botond,tthibaud,android-reviewers
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 130 Branch
Regressions: 1911198
Regressions: 1911188
Blocks: 1903682
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: