GeckoView should not ask APZ for an InputResult for events other than MotionEvent.ACTION_DOWN
Categories
(Fenix :: Browser Engine, task, P2)
Tracking
(firefox130 fixed)
Tracking | Status | |
---|---|---|
firefox130 | --- | fixed |
People
(Reporter: botond, Assigned: hiro)
References
(Blocks 3 open bugs)
Details
Attachments
(1 file)
Bug 1897567 - Invoke onTouchEventForDetailResult just once for each touch block. r?botond!,tthibaud!
48 bytes,
text/x-phabricator-request
|
Details | Review |
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.
Assignee | ||
Comment 1•6 months ago
|
||
My best guess here is probably we need to do the same thing what this code block does in the MotionEvent.ACTION_DOWN case.
Assignee | ||
Comment 2•5 months ago
|
||
Assignee | ||
Comment 3•5 months ago
|
||
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. :/
Updated•5 months ago
|
Updated•5 months ago
|
Updated•4 months ago
|
Comment 4•4 months ago
|
||
(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?
Assignee | ||
Comment 5•4 months ago
|
||
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.
Comment 7•4 months ago
|
||
bugherder |
Description
•