1227241 - Tapping on links in Fennec-APZ doesn't seem to visibly activate them

Status: NEW

(Core :: Panning and Zooming, defect, P3)

Description

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

On my bugmail dashboard (there's a static snapshot at http://people.mozilla.org/~kgupta/bugmash.html) clicking on the 'X' in the corner of the bug blocks used to make the red for a split second as they got activated. With the C++ APZ they don't seem to do that any more. Not sure if they're not getting activated at all, or if the activation happens too late, after the block is deleted (that's what clicking on the X does).

Comment 1

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

Preliminary investigation seems to indicate that the touchstart/touchend WidgetEvent instances reach the APZEventState before the GeckoContentController state change notifications. So at the point that ActiveElementManager::HandleTouchEndEvent is called, the mEndTouchIsClick variable hasn't been populated and so it doesn't activate the link properly. I'll look a bit deeper.

Comment 2

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

I think this is partly a regression from bug 1141127. Or at least, that bug made the situation worse, because it delayed the content-response notifications from the main thread (by eating the touchmoves that might have triggered the response) and therefore delays the handling of the input block in the APZ. Therefore the StartTouch notification to the GeckoContentController is delayed, and we don't trigger the element activation as early as we used to.

That gets compounded with what I described in comment 1 in the case of a tap. Previously, on some devices, we were almost certain to get a touch-move during a tap, so the input block would get processed on the APZ side (and the EndTouch GeckoContentController notification would get dispatched) before the touch-end WidgetInputEvent arrived on the main thread. Now, however, in the case of a tap, the touch-end WidgetInputEvent arrives on the main thread first, and the content response from that is what triggers the processing of the input block on the APZ side, so the EndTouch notification arrives later.

All of this could happen previously as well, so it's not a strict regression, but it does manifest a lot more frequently now. I think we need to update the ActiveElementManager code to deal with the new flow of events better, and not rely on the GeckoContentController state change notifications as much.

Comment 3

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

After playing around with this more I think one of the fundamental issues I'm running to is that the single-tap notification now almost always arrives after the endtouch. This means the AEM has already cleared its internal state (i.e. which element the target is) and so when we process the single-tap we don't know what to activate without doing another hit test (which I'd like to avoid). Still trying to figure out the best way around this.

Comment 4

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

I'm not actively working on this, throwing it back into the pool.

Comment 5

[Randall Barker [:rbarker]]

Seems to be working for me on a Nexus 4. Here was the log output from a successful active style change:

I/Gecko   (15787): AEM: Setting target element to 0x83ac9820
I/Gecko   (15787): AEM: Touch end event, aWasClick: 1
I/Gecko   (15787): AEM: Cancelling task 0x0
I/Gecko   (15787): AEM: Setting active 0x83ac9820
I/Gecko   (15787): AEM: Element 0x83ac9820's style is dependent on the active state
I/Gecko   (15787): AEM: Touch start, aCanBePan: 1
I/Gecko   (15787): AEM: Touch end, clearing pan state
I/Gecko   (15787): AEM: Setting target element to 0x85b03da0
I/Gecko   (15787): AEM: Touch end event, aWasClick: 1
I/Gecko   (15787): AEM: Cancelling task 0x0
I/Gecko   (15787): AEM: Setting active 0x85b03da0
I/Gecko   (15787): AEM: Element 0x85b03da0's style is dependent on the active state
I/Gecko   (15787): AEM: Touch start, aCanBePan: 1
I/Gecko   (15787): AEM: Touch end, clearing pan state

Leaving open so it maybe debugged at a later time.

Comment 6

[Botond Ballo [:botond]]

See bug 1256344 comment 6 for an explanation of what may be causing (or contributing to) this.

Comment 7

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

When we fix this bug (presumably by overhauling the APZEventState machinery to track things better) we should make sure we also do this for the mTouchRollup field I'm adding in bug 1343977. See the discussion at bug 1343977 comment 7 and onwards.