1037409 - Remove touch-action gtests that don't make sense

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

There are a few gtests that use "default gestures" (i.e. don't use the APZC's gesture detector, but send in gesture events directly) and that also set allowed touch behavior for touch-action. I don't think these tests make sense because this will never happen in the real world.

If we sending pinch events directly into the APZC, that means we cannot reliably also be sending touch behaviour. For example on Mac doing a pinch on the trackpad can generate a pinch with the correct focus point, but you cannot reliably map the user's fingers to two touch points for which you can determine touch behaviour. I think that if the APZC gets a pinch gesture event from the widget code, it must just handle it unconditionally. If the platform wants to enforce touch-action behavior in this case, then it should send in the individual touch points rather than the pre-detected PinchGestureEvent.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 2

[Nick Lebedev [:nl]]

Comment on attachment 8454428 [details] [diff] [review]
Patch

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

>> For example on Mac doing a pinch on the trackpad can generate a pinch with the correct focus point, but you cannot reliably map the user's fingers to two touch points for which you can determine touch behaviour. I think that if the APZC gets a pinch gesture event from the widget code, it must just handle it unconditionally.

If it is a real world use-case then we might need to make sure that Apzc's code will handle it correctly. I believe it will because if Apzc is provided with the Pinch Gestures [1] it won't start WaitingContentResponse timer and will check in the [2] an empty array which will return true. But it's preliminary thoughts.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#903
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#2528

About tests - I agree, it's ok to remove them.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

(In reply to Nick Lebedev [:nl] from comment #2)
> If it is a real world use-case then we might need to make sure that Apzc's
> code will handle it correctly. I believe it will because if Apzc is provided
> with the Pinch Gestures [1] it won't start WaitingContentResponse timer and
> will check in the [2] an empty array which will return true. But it's
> preliminary thoughts.

I agree that we need to make sure that APZC handles this correctly. But right now the code is a mess and I'm in the process of cleaning it up. I'll be adding more tests that make sense as part of the cleanup. See bug 1009733 and the bugs it depends on for more details.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a497072902b

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3a497072902b