1041451 - Still receive long tap up event even though long tap event is handled.

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Consider follow sequence

     TouchStart----------->FireLongTap-------------->TouchEnd


TouchStart would start a new touch block and then +1 to mTouchBlockBalance.

Next, long tap would be fired. Long tap also start a new touch block.
When content received long tap, it will call SendContentReceivedTouch().
So that, chrome will minus 1 to mTouchBlockBalance. mTouchBlockBalance is 0 now which mean the touch block created by TouchStart is end here.

Next, Touch end fired and call SendContentReceivedTouch() again. chrome minus 1 to mTouchBlockBalance again. Right now balance is -1 so it will find next touch block. This touch block is created by long tap.

So I found that defaultPrevent doesn't send to correct touch block. The defaultPrevent generate by long tap event send to touch block that is created by TouchStart and vice versa. So long tap up event still fire even though long tap is handled.

Comment 1

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: bug1041451 v1

If we have pending response in RecvHandleLongTap, submit this response first. So that we can end touch block which is generated by TouchStart.

Comment 2

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

Comment on attachment 8459489 [details] [diff] [review]
bug1041451 v1

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

Good catch, thanks for tracking that down! I'd like to see two changes to this patch though. One is to pull this block out into a helper method:

void TabChild::SendPendingTouchPreventedResponse(bool aPreventDefault) {
  if (mPendingTouchPreventedResponse) {
...
}

and then call this helper method from RecvHandleLongTap and also from the TOUCH_MOVE handler in RecvRealTouchEvent.

The second change is to move this code higher up in RecvHandleLongTap, to before the dispatch of the contextmenu event. With those two changes it should be good but I'd like to see the updated patch for review.

Comment 3

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: bug1041451 v2

Thanks for review! I have updated patch to address comment.

Comment 4

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

Comment on attachment 8459974 [details] [diff] [review]
bug1041451 v2

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

Looks good, thanks!

Comment 5

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: bug1041451 v3 (carry r+: kats)

Rebased to latest m-c

try run: https://tbpl.mozilla.org/?tree=Try&rev=326dfafbc940

Comment 6

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

try run looks good, ready for check-in.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c452b1384965

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c452b1384965

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c452b1384965