Bug 1685648 Comment 17 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Botond Ballo [:botond] from comment #14)
> (In reply to Razvan Cojocaru from comment #13)
> > EDIT: I'm guessing something along [these lines](https://searchfox.org/mozilla-central/source/gfx/layers/apz/test/gtest/TestPinching.cpp#288)?
> 
> Yep, that's a good one to take inspiration from!
> 
> Since the steps to reproduce involve the pinch having a particular shape (mostly vertical movement of the touch points), instead of calling `PinchWithTouchInput()` you probably want to just send individual touch events (as in the implementation of `PinchWithTouchInput()`) directly from the test, with the coordinates chosen to produce this shape.

I've looked into this, and I wonder if we wouldn't benefit more from just passing a `bool vertical = false` extra parameter to `PinchWithTouchInput()` and make it pinch vertically in that case. As for your other comment, I'm certainly a relative novice with the Firefox code, but I see that there's a `PinchOptions aOptions = PinchOptions::LiftBothFingers` parameter to `PinchWithTouchInput()` [here](https://searchfox.org/mozilla-central/source/gfx/layers/apz/test/gtest/APZTestCommon.h#557), maybe we could add something like `LiftBothFingersInSequence`?

If you prefer that I just duplicate the relevant logic in the test and don't modify `PinchWithTouchInput()` I'll be happy to do that as well.
(In reply to Botond Ballo [:botond] from comment #14)
> (In reply to Razvan Cojocaru from comment #13)
> > EDIT: I'm guessing something along [these lines](https://searchfox.org/mozilla-central/source/gfx/layers/apz/test/gtest/TestPinching.cpp#288)?
> 
> Yep, that's a good one to take inspiration from!
> 
> Since the steps to reproduce involve the pinch having a particular shape (mostly vertical movement of the touch points), instead of calling `PinchWithTouchInput()` you probably want to just send individual touch events (as in the implementation of `PinchWithTouchInput()`) directly from the test, with the coordinates chosen to produce this shape.

I've looked into this, and I wonder if we wouldn't benefit more from just passing a `bool vertical = false` extra parameter to `PinchWithTouchInput()` and make it pinch vertically in that case. As for your other comment, I see that there's a `PinchOptions aOptions = PinchOptions::LiftBothFingers` parameter to `PinchWithTouchInput()` [here](https://searchfox.org/mozilla-central/source/gfx/layers/apz/test/gtest/APZTestCommon.h#557), maybe we could add something like `LiftBothFingersInSequence`?

If you prefer that I just duplicate the relevant logic in the test and don't modify `PinchWithTouchInput()` I'll be happy to do that as well.

Back to Bug 1685648 Comment 17