Closed Bug 1013378 Opened 6 years ago Closed 6 years ago

Subsequent taps after first one do not trigger :active state

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: botond, Assigned: nl)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Blocks: 964750
Nick, please fix or back out bug 964750.
Assignee: nobody → nicklebedev37
See also: Bug 1013556 - FxOS 'click' events don't match :active element
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.
Attachment #8426988 - Flags: review?(bugmail.mozilla)
(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!!
try results: https://tbpl.mozilla.org/?tree=Try&rev=96d1235be671 (seems to be good).
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.
Attachment #8426988 - Flags: review?(bugmail.mozilla) → review+
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
Addressed botond's comment.
The patch is ready to be landed.
Attachment #8426988 - Attachment is obsolete: true
Attachment #8427265 - Flags: review+
Green try build including gtests is in comment 6; setting checkin-needed since inbound is closed.
Keywords: checkin-needed
Blocks: 1014838
https://hg.mozilla.org/mozilla-central/rev/639758e9e5ab
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.