1013378 - Subsequent taps after first one do not trigger :active state

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Botond Ballo [:botond]]

STR:
  1. Load http://jsbin.com/vorib in the B2G browser
  2. Tap one of the buttons. The buttons gets highlighted as you tap it - so far, so god.
  3. Tap a second button.

Expected:
  The second button is highlighted as you tap it.

Actual:
  The second button is not highlighted.

Subsequent taps also do not cause highlights, though after lots of tapping you occasionally see a highlight sometimes.

Comment 1

[Botond Ballo [:botond]]

It looks like AsyncPanZoomController::OnTouchEnd() is no longer being called for the touch-end of a tap, and therefore the APZStateChange::EndTouch message which is needed to maintain correct state in ActiveElementManager is never sent.

This is a regression from bug 964750, which introduced an early return from HandleInputEvent() for the touch-end event of a tap at http://dxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?from=AsyncPanZoomController.cpp#640.

Comment 2

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

Nick, please fix or back out bug 964750.

Comment 3

[Justin D'Arcangelo [:justindarc]]

See also: Bug 1013556 - FxOS 'click' events don't match :active element

Comment 4

[Nick Lebedev [:nl]]

Attachment: Make GestureEventListener not to throw apzc status up for touchend event since it causes apzc stay in touching state after gestures completed.

First version of the patch.
I need to perform some more testing but i believe patch won't change much.
Hope to complete it today.

Comment 5

[Justin D'Arcangelo [:justindarc]]

(In reply to Nick Lebedev [:nl] from comment #4)
> Created attachment 8426988 [details] [diff] [review]
> Make GestureEventListener not to throw apzc status up for touchend event
> since it causes apzc stay in touching state after gestures completed.
> 
> First version of the patch.
> I need to perform some more testing but i believe patch won't change much.
> Hope to complete it today.

Much appreciated!!

Comment 6

[Nick Lebedev [:nl]]

try results: https://tbpl.mozilla.org/?tree=Try&rev=96d1235be671 (seems to be good).

Comment 7

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

Comment on attachment 8426988 [details] [diff] [review]
Make GestureEventListener not to throw apzc status up for touchend event since it causes apzc stay in touching state after gestures completed.

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

This looks ok to me. We don't much that makes sense with these return values anyway. I'm planning on rewriting some of this code in bug 1009733 so this is fine for now.

Comment 8

[Botond Ballo [:botond]]

Comment on attachment 8426988 [details] [diff] [review]
Make GestureEventListener not to throw apzc status up for touchend event since it causes apzc stay in touching state after gestures completed.

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

::: gfx/layers/apz/src/GestureEventListener.cpp
@@ +274,4 @@
>  
>  nsEventStatus GestureEventListener::HandleInputTouchEnd()
>  {
> +  // We intentionally do not throw apzc return statues up since

s/throw/pass, s/statues/statuses

Comment 9

[Nick Lebedev [:nl]]

Attachment: Make GestureEventListener not to throw apzc status up for touchend event since it causes apzc stay in touching state after gestures completed. v2.

Addressed botond's comment.
The patch is ready to be landed.

Comment 10

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

Green try build including gtests is in comment 6; setting checkin-needed since inbound is closed.

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/639758e9e5ab

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/639758e9e5ab