964716 - Inconsistency in returning nsEventStatus result by apzc when handling pinch

Status: RESOLVED INVALID

(Core :: Panning and Zooming, defect)

Description

[Nick Lebedev [:nl]]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.76 Safari/537.36

Comment 1

[Nick Lebedev [:nl]]

In this bug I would like to note a potential problem in the apzctm/apzc that connected with the nsEventStatus returned by apzc in different conditions.

If I supply two touchstart events (that represent a pinch) to the apzc i’m getting different nsEventStatus results depending whether apz has (or doesn’t have) mMayHaveTouchListeners set to true.
In case there are mMayHaveTouchListeners set to true i’m getting nsEventStatus_eIgnore result for both touchstart events.
In another case when there are no touchlisteners i’m getting nsEventStatus_eConsumeNoDefault results for both touchstart events. Stacktraces for both cases are attached.

Currently such issue doesn’t break anything but affecting new coming pointer events since widget code that generates pointer events relies on the apzc returned statuses. If I document has touch listeners and I try to pinch i get pointerdown/pointerdown/pointerup/pointerup events, if there are no touch listeners - pointerdown/pointerdown/pointercancel/pointercancel. But of course pointer events shouldn’t be affected by absence of touch listeners.

I believe we would like to have the same outputs in both cases for consistency purposes.

For now we can mitigate these difference by ignoring returned status for touchstart events in the widget code but please notify in this bug if there are intentions to rework apzc side.

Comment 2

[Nick Lebedev [:nl]]

Attachment: with_touchlisteners.txt

Stack trace of aspz's touchstart handling when there are touch listeners in the document.

Comment 3

[Nick Lebedev [:nl]]

Attachment: without_touchlisteners.txt

Stack trace of aspz's touchstart handling when there are NO touch listeners in the document.

Comment 4

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

Can you describe how the pointer events stuff uses the return value from ReceiveInputEvent? I'm planning to make a bunch of changes to that return value and I want to understand how pointer events uses it so that I can make sure those use cases are supported.

Comment 5

[Nick Lebedev [:nl]]

Actually pointer events don't interact with the return value from ReceiveInputEvent because they are generated from touches on the higher level then widget [1]. So it's better to describe how touch events interact with the return value.
As far as I remember the metro widget (not sure about other widgets) used the touches in the following way: it ignored return result from the TouchStart event and because of this everything worked fine in spite of the inconsistency in the returned results when TouchListeners are presented and when not. Then it checks every time the return result of the TouchMove events and if TouchMove on some step is equal to the nsEventStatus_eConsumeDoDefault then it dispatches TouchCancel to content and stops dispatching any further events to content and proceeds dispatching events to the Apzc.

Hope this helps, feel free to ping me if there are some more questions.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?force=1#6686

Comment 6

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

Ok, if the pointer events code doesn't depend on the return value from ReceiveInputEvent, then this:

(In reply to Nick Lebedev [:nl] from comment #1)
> Currently such issue doesn’t break anything but affecting new coming pointer
> events since widget code that generates pointer events relies on the apzc
> returned statuses.

sounds like it is no longer valid? In that case I'm going to resolve this bug as INVALID. Please reopen if you think there is still something that needs to be done here.

Also note that as part of bug 998025 we changed entirely what the return value of APZCTreeManager::ReceiveInputEvent means. If there is any impact to pointer events because of that we can use bug 1036985 to track that.