1029447 - [Vertical Homescreen] After pinch-to-zoom gesture neither click nor contextmenu events are dispatched in the first attempt

Status: RESOLVED WORKSFORME

(Core :: Panning and Zooming, defect)

Description

[Cristian Rodriguez (:crdlc)]

STR:

(I know that we don't have zoom gesture but this does not mean that an user can do this gesture)

1) Go to vertical homescreen
2) Do pinch-to-zoom gesture with two fingers
3) After the gesture try to open an app or to show the context menu

Expected result:

The app is opened or the contextmenu is displayed

Current state:

"click" and "contextmenu" event are not dispatched after the gesture

It is very easy to recover the homescreen from the bug because it only happens the first time, if you try to open an app again and to show the contextmenu it will work fine. It only happens the first attempt. Consecutive attempts work fine.

I noticed that the pinch-to-zoom gesture dispatchs a "contextmenu" event in https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_dragdrop.js#L354. It is a bit weird IMHO. I was trying to find a solution but I don't have any idea how to fix it :(

Comment 1

[Cristian Rodriguez (:crdlc)]

Any idea friends? I am completely lost here

Comment 2

[Chris Lord [:cwiiis]]

I'm free at the moment, so I'll take this if that's ok.

Comment 3

[Chris Lord [:cwiiis]]

My debugging so far leads me to believe this is a platform issue, I'm just having a look through the related Gecko code to see what's going on.

Comment 4

[Chris Lord [:cwiiis]]

This appears to be a bug in the AsyncPanZoomController. Still debugging, I'm thinking it'll probably be a reasonably simple fix.

Comment 5

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

I've also noticed that the first tap after opening an app doesn't seem to work

Comment 6

[Chris Lord [:cwiiis]]

I've instrumented GestureEventListener to help debug this;

This is the sequence of events for a pinch:

I/Gecko   ( 4466): XXX HandleInputTouchMultiStart
I/Gecko   ( 4466): XXX SetState(GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX SetState(GESTURE_PINCH)
I/Gecko   ( 4466): XXX HandleInputTouchMove
...
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchEnd
I/Gecko   ( 4466): XXX SetState(GESTURE_NONE)
I/Gecko   ( 4466): XXX HandleInputTouchSingleStart
I/Gecko   ( 4466): XXX SetState(GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   ( 4466): XXX CreateLongTapTimeoutTask
I/Gecko   ( 4466): XXX CreateMaxTapTimeoutTask
I/Gecko   ( 4466): XXX HandleInputTouchEnd
I/Gecko   ( 4466): XXX HandleInputTimeoutMaxTap
I/Gecko   ( 4466): XXX SetState(GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN)
I/Gecko   ( 4466): XXX HandleInputTimeoutLongTap
I/Gecko   ( 4466): XXX SetState(GESTURE_LONG_TOUCH_DOWN)
I/Gecko   ( 4466): XXX Sending Long-press


If you do a pinch after this, the sequence is slightly different:

I/Gecko   ( 4466): XXX HandleInputTouchMultiStart
I/Gecko   ( 4466): XXX SetState(GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX SetState(GESTURE_PINCH)
I/Gecko   ( 4466): XXX HandleInputTouchMove
...
I/Gecko   ( 4466): XXX HandleInputTouchMove
I/Gecko   ( 4466): XXX HandleInputTouchEnd
I/Gecko   ( 4466): XXX SetState(GESTURE_NONE)
I/Gecko   ( 4466): XXX HandleInputTouchEnd
I/Gecko   ( 4466): XXX HandleInputTouchSingleStart
I/Gecko   ( 4466): XXX SetState(GESTURE_NONE)

This difference between these two is that the first is missing a HandleInputTouchEnd before the last HandleInputTouchSingleStart... But the state is still GESTURE_NONE before HandleInputTouchSingleStart, so that shouldn't really matter... Either way, the issue here seems to be the HandleInputTouchSingleStart after lifting the pinch, so I guess there's something wrong in the logic somewhere. It shouldn't take too long to find.

Comment 7

[Chris Lord [:cwiiis]]

The same sequences with more instrumentation:

I/Gecko   ( 5610): XXX HandleInputTouchMultiStart (GESTURE_NONE)
I/Gecko   ( 5610): XXX SetState from GESTURE_NONE to GESTURE_MULTI_TOUCH_DOWN
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX SetState from GESTURE_MULTI_TOUCH_DOWN to GESTURE_PINCH
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_PINCH)
...
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_PINCH)
I/Gecko   ( 5610): XXX HandleInputTouchEnd (GESTURE_PINCH)
I/Gecko   ( 5610): XXX SetState from GESTURE_PINCH to GESTURE_NONE
I/Gecko   ( 5610): XXX HandleInputTouchSingleStart (GESTURE_NONE)
I/Gecko   ( 5610): XXX SetState from GESTURE_NONE to GESTURE_FIRST_SINGLE_TOUCH_DOWN
I/Gecko   ( 5610): XXX CreateLongTapTimeoutTask (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   ( 5610): XXX CreateMaxTapTimeoutTask (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   ( 5610): XXX HandleInputTouchEnd (GESTURE_NONE)
I/Gecko   ( 5610): XXX HandleInputTimeoutMaxTap (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   ( 5610): XXX SetState from GESTURE_FIRST_SINGLE_TOUCH_DOWN to GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN
I/Gecko   ( 5610): XXX HandleInputTimeoutLongTap (GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN)
I/Gecko   ( 5610): XXX SetState from GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN to GESTURE_LONG_TOUCH_DOWN
I/Gecko   ( 5610): XXX Long-press



I/Gecko   ( 5610): XXX HandleInputTouchMultiStart (GESTURE_NONE)
I/Gecko   ( 5610): XXX SetState from GESTURE_NONE to GESTURE_MULTI_TOUCH_DOWN
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   ( 5610): XXX SetState from GESTURE_MULTI_TOUCH_DOWN to GESTURE_PINCH
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_PINCH)
...
I/Gecko   ( 5610): XXX HandleInputTouchMove (GESTURE_PINCH)
I/Gecko   ( 5610): XXX HandleInputTouchEnd (GESTURE_PINCH)
I/Gecko   ( 5610): XXX SetState from GESTURE_PINCH to GESTURE_NONE
I/Gecko   ( 5610): XXX HandleInputTouchEnd (GESTURE_NONE)
I/Gecko   ( 5610): XXX HandleInputTouchSingleStart (GESTURE_LONG_TOUCH_DOWN)
I/Gecko   ( 5610): XXX SetState from GESTURE_LONG_TOUCH_DOWN to GESTURE_NONE


The extra HandleInputTouchEnd call would indicate that the error is coming from outside of GestureEventListener, as this is only called in response to a HandleInputEvent call with MultiTouchInput::MULTITOUCH_END/LEAVE.

Comment 8

[Chris Lord [:cwiiis]]

Adding object pointers to the output makes this a bit more illuminating:

First pinch:

I/Gecko   (12592): XXX (0xa9314000) Waiting for content response
I/Gecko   (12592): XXX (0xa9314000) Adding event to queue (current length: 0)
I/Gecko   (12592): XXX (0xa9313800) Waiting for content response
I/Gecko   (12592): XXX (0xa9313800) Adding event to queue (current length: 0)
I/Gecko   (12592): XXX (0xa9313800) Adding event to queue (current length: 1)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMultiStart (GESTURE_NONE)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_NONE to GESTURE_MULTI_TOUCH_DOWN
I/Gecko   (12592): XXX (0xa9313800) Removing event from queue (Current length: 2)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa9313800) Removing event from queue (Current length: 1)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_MULTI_TOUCH_DOWN to GESTURE_PINCH
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_PINCH)
...
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_PINCH)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchEnd (GESTURE_PINCH)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_PINCH to GESTURE_NONE
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchEnd (GESTURE_NONE)
I/Gecko   (12592): XXX (0xa906ee70) HandleInputTouchSingleStart (GESTURE_NONE)
I/Gecko   (12592): XXX (0xa906ee70) SetState from GESTURE_NONE to GESTURE_FIRST_SINGLE_TOUCH_DOWN
I/Gecko   (12592): XXX (0xa906ee70) CreateLongTapTimeoutTask (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ee70) CreateMaxTapTimeoutTask (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa9314000) Removing event from queue (Current length: 1)
I/Gecko   (12592): XXX (0xa906ee70) HandleInputTimeoutMaxTap (GESTURE_FIRST_SINGLE_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ee70) SetState from GESTURE_FIRST_SINGLE_TOUCH_DOWN to GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN
I/Gecko   (12592): XXX (0xa906ee70) HandleInputTimeoutLongTap (GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN)
I/Gecko   (12592): XXX (0xa906ee70) SetState from GESTURE_FIRST_SINGLE_TOUCH_MAX_TAP_DOWN to GESTURE_LONG_TOUCH_DOWN
I/Gecko   (12592): XXX (0xa9314000) OnLongPress


Second pinch:

I/Gecko   (12592): XXX (0xa9314000) Waiting for content response
I/Gecko   (12592): XXX (0xa9314000) Adding event to queue (current length: 0)
I/Gecko   (12592): XXX (0xa9313800) Waiting for content response
I/Gecko   (12592): XXX (0xa9313800) Adding event to queue (current length: 0)
I/Gecko   (12592): XXX (0xa9313800) Adding event to queue (current length: 1)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMultiStart (GESTURE_NONE)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_NONE to GESTURE_MULTI_TOUCH_DOWN
I/Gecko   (12592): XXX (0xa9313800) Removing event from queue (Current length: 2)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa9313800) Removing event from queue (Current length: 1)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_MULTI_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_MULTI_TOUCH_DOWN to GESTURE_PINCH
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_PINCH)
...
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchMove (GESTURE_PINCH)
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchEnd (GESTURE_PINCH)
I/Gecko   (12592): XXX (0xa906ea60) SetState from GESTURE_PINCH to GESTURE_NONE
I/Gecko   (12592): XXX (0xa906ea60) HandleInputTouchEnd (GESTURE_NONE)
I/Gecko   (12592): XXX (0xa906ee70) HandleInputTouchSingleStart (GESTURE_LONG_TOUCH_DOWN)
I/Gecko   (12592): XXX (0xa906ee70) SetState from GESTURE_LONG_TOUCH_DOWN to GESTURE_NONE
I/Gecko   (12592): XXX (0xa9314000) Removing event from queue (Current length: 1)


So it seems in this situation that the GestureListener 0x906ee70 is left in an inconsistent state after the first pinch. In fact we can see that it gets left in GESTURE_LONG_TOUCH_DOWN and isn't reset to GESTURE_NONE. We can also see this seems to be because it only gets a touch-down, but not a touch-up.

At this point, I must concede that the bug isn't as simple as I thought it would be. I'm unassigning from myself for now as this is outside of my domain and there are other pressing bugs I can deal with more expediently, but needinfo'ing kats to see who can pick this up.

Comment 9

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

Please nom for blocking-b2g status if so desired (and indicate which branches are affected), and I'll prioritize it accordingly.

Comment 10

[Chris Lord [:cwiiis]]

Although I don't think this particular manifestation of the bug warrants blocking status, I've noticed that the first touch after newly launching an app also goes missing, which is a more serious bug (especially for testability).

Nominating this for blocking, based on it negatively affecting our ability to test, not on the user impact (which I think is annoying, especially for the browser, but small). I won't be too upset if that isn't reason enough to consider this blocking (but QA might be? :))

Comment 11

[Jason Smith [:jsmith]]

(In reply to Chris Lord [:cwiiis] from comment #10)
> Although I don't think this particular manifestation of the bug warrants
> blocking status, I've noticed that the first touch after newly launching an
> app also goes missing, which is a more serious bug (especially for
> testability).

According to Milan, this might be a separate problem. Can you file this separately?

> 
> Nominating this for blocking, based on it negatively affecting our ability
> to test, not on the user impact (which I think is annoying, especially for
> the browser, but small). I won't be too upset if that isn't reason enough to
> consider this blocking (but QA might be? :))

Given that pinch to zoom isn't likely on the homescreen & a second click will allow interaction, I don't think we should block.

Comment 12

[Chris Lord [:cwiiis]]

Filed bug 1030172.

Comment 13

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

Can anybody still reproduce this on master or 2.0? I tried just now but it seems to have been fixed. Was there a recent change to the homescreen that maybe removed touch listeners?

Comment 14

[Kevin Grandon :kgrandon]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #13)
> Can anybody still reproduce this on master or 2.0? I tried just now but it
> seems to have been fixed. Was there a recent change to the homescreen that
> maybe removed touch listeners?

We still have the listeners, but it seems to work for me now. Closing, someone reopen if you still see it.