Botond Ballo [:botond] Reporter
	Description • 11 years ago

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.

	Botond Ballo [:botond] Reporter
	Comment 1 • 11 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 2 • 11 years ago

Nick, please fix or back out bug 964750.

	Justin D'Arcangelo [:justindarc]
	Comment 3 • 11 years ago

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

	Nick Lebedev [:nl] Assignee
	Comment 4 • 10 years ago

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.

	Justin D'Arcangelo [:justindarc]
	Comment 5 • 10 years ago

(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!!

	Nick Lebedev [:nl] Assignee
	Comment 6 • 10 years ago
green try

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 7 • 10 years ago

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.

	Botond Ballo [:botond] Reporter
	Comment 8 • 10 years ago

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

	Nick Lebedev [:nl] Assignee
	Comment 9 • 10 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 10 • 10 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 11 • 10 years ago

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

	Carsten Book [:Tomcat]
	Comment 12 • 10 years ago

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