Vertical scroll gestures are sometimes ignored when scrolling on container that can scroll horizontally
Categories
(Core :: Panning and Zooming, defect, P3)
Tracking
()
People
(Reporter: cpeterson, Assigned: kats)
References
(Blocks 1 open bug)
Details
(Whiteboard: [geckoview:fenix:m4])
Attachments
(2 files)
@ Botond, does this Android scrolling issue sound familiar to you? Is this a problem with Gecko's X/Y-axis scroll "locking"?
This bug was originally reported in the Fenix issue tracker:
https://github.com/mozilla-mobile/fenix/issues/1212
Steps to reproduce
- Go to https://en.m.wikipedia.org/wiki/List_of_pastries
- Scroll the page down, including moving your finger on the table of pastries to continue scrolling
Expected behavior
Each downward scroll gesture is registered.
Actual behavior
Downward scroll gestures are sometimes dropped.
Assignee | ||
Comment 1•5 years ago
|
||
Interesting. I can reproduce some super-duper-fast scrolling, but only if I first horizontally scroll the pastries table (presumably activating that scrollframe is important to the STR). Once the displayport on the subscroller expires the behaviour goes back to normal.
Assignee | ||
Comment 2•5 years ago
|
||
Also interesting is that I can reproduce on a Pixel 2 with Fenix, but not on my Xperia XZ1C in either Fennec or Fenix.
Assignee | ||
Comment 3•5 years ago
|
||
Also see it on Fennec on a Pixel 2. So probably the Pixel 2 is sending us some touch event sequence that we're not handling super well.
Comment 4•5 years ago
|
||
I haven't been able to repro it on my Moto G5 (tested both Fennec and Fenix), supporting the hypothesis of a Pixel-specific issue.
Assignee | ||
Comment 5•5 years ago
|
||
I'll investigate a bit here since I can repro on my Pixel 2
Assignee | ||
Comment 6•5 years ago
|
||
In the end the cause here for the issue in comment 0 (vertical scroll actions get dropped) is rather boring. The pastries table is scrollable vertically by 0.03125 pixels, which means IsZero returns false on it even though visually it doesn't really move. That combined with immediate handoff being disabled means that every time you change scroll directions on the table it will consume the pan attempt by scrolling the pastries table vertically and not handing off anything. Subsequent scrolls then work.
I also found that the fling acceleration code was sometimes accelerating x=0 flings into x > 0 flings which seems wrong, so I wrote a little patch to fix that. I'll spin that out into a separate bug though.
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
When I first started investigating this issue I was also seeing a problem where sometimes the fling would go unexpectedly fast, bringing me to the end of the page in a second or so. I saw it once with logging enabled, and it seemed like the fling velocity was continually increasing instead of decreasing with friction. However with the patches in the above try push I can't seem to reproduce that problem any more, so unless I see it again I'm not going to try investigating that further.
Assignee | ||
Comment 9•5 years ago
|
||
That try push has oranges, because I missed the lock ordering. https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=96b97b8c908e5c00e7faee97b57159db066541e5 should be better
Assignee | ||
Comment 10•5 years ago
|
||
Assignee | ||
Comment 11•5 years ago
|
||
Depends on D26924
Comment 12•5 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/581ae96f555e Add some more logging statements. r=botond https://hg.mozilla.org/integration/autoland/rev/7d29466fd0c1 Allow immediate handoff if APZC's displacement is not a user visible amount. r=botond
Comment 13•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/581ae96f555e
https://hg.mozilla.org/mozilla-central/rev/7d29466fd0c1
Reporter | ||
Comment 14•5 years ago
|
||
Kats, do you think we should uplift this scrolling fix to Fennec 67 Beta? How common is this case?
Assignee | ||
Comment 15•5 years ago
|
||
I would lean towards letting it ride the trains. I think the scenario is pretty uncommon and (as evidenced above) device-specific. While the fix is conceptually simple, the implementation and surrounding code seem like they might potentially impact other use cases and the extra bake time would be good to have.
Reporter | ||
Comment 16•5 years ago
|
||
I would lean towards letting it ride the trains.
Sounds good. 67=wontfix, in that case.
Description
•