Closed Bug 1083398 Opened 5 years ago Closed 5 years ago

Tapping while flinging fast may still trigger a tap if the fling is higher up in the handoff chain

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files)

Found this while working on bug 1083395. I haven't verified it can actually manifest in practice, but the code at [1] is intended to consume a tap to stop a fling rather than dispatching the tap through to content. However the code only checks the velocity of the current APZ, while in fact there might be a fling on an APZ higher up in the handoff chain. The CancelAnimations is done on the entire handoff chain so the fling will be stopped, but the tap will still go through.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=c0e4ec5fc709#1031
Attachment #8505686 - Flags: review?(botond) → review+
Comment on attachment 8505687 [details] [diff] [review]
Part 2 - Check the entire handoff chain for fast-moving APZCs

Review of attachment 8505687 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +2332,5 @@
>  
> +bool AsyncPanZoomController::IsMovingFast() const {
> +  ReentrantMonitorAutoEnter lock(mMonitor);
> +  return (GetVelocityVector().Length() > gfxPrefs::APZFlingStopOnTapThreshold());
> +}

I can imagine someone reading this method without knowing how it's ultimately used being confused about why "moving fast" is defined as "moving faster than the fling-stop-on-tap threshold", but the alternatives that I can think of (calling the method IsMovingFasterThanFlingStopOnTapThreshold(), or calling it IsMovingFasterThan() and making the threshold a parameter) are unwieldy, so let's go with this. We can probably add a comment in the header, though.
Attachment #8505687 - Flags: review?(botond) → review+
https://hg.mozilla.org/mozilla-central/rev/ebf448375747
https://hg.mozilla.org/mozilla-central/rev/d05cd2481d96
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.