859951 - Refactor the code to convert from MOTION_EVENT to nsTouchEvent instances

Status: RESOLVED FIXED

(Core :: Widget: Android, defect)

Description

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

Attachment: Refactor event creation code

This I think reduces the code somewhat, and more importantly, allows me to reuse some of this for creating the input events I'll need for AsyncPanZoomController hookup.

This patch should have no functional changes.

Comment 1

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

Attachment: Use getActionMasked() and getActionIndex() more

Also going to throw this on here. Again, no functional changes intended.

Comment 2

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

Attachment: (1/2) Refactor event creation code

Rebased, fixed bug number

Comment 3

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

Attachment: (2/2) Use getActionMasked() and getActionIndex()

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 736270 [details] [diff] [review]
(1/2) Refactor event creation code

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

::: widget/android/nsWindow.cpp
@@ +1202,5 @@
>      bool preventDefaultActions = false;
>      bool isDownEvent = false;
> +
> +    nsTouchEvent event = ae->MakeTouchEvent(this);
> +    if (event.message != NS_EVENT_NULL) {

Just to ask, are you sure we want to keep going if NS_EVENT_NULL at all? Maybe it'd be safer to just return early. Or maybe we have things we want to do later that should happen regardless...

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 736271 [details] [diff] [review]
(2/2) Use getActionMasked() and getActionIndex()

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

nice!

Comment 6

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

(In reply to Wesley Johnston (:wesj) from comment #4)
> Just to ask, are you sure we want to keep going if NS_EVENT_NULL at all?
> Maybe it'd be safer to just return early. Or maybe we have things we want to
> do later that should happen regardless...

I looked briefly at what happens if we don't return early, and I think we do want to keep it this way because of the prevent-default notification. If we miss that notification then things have the potential to go bad so I think it's safer to run that code if we have an unknown event type.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/b978b9e5c2df
https://hg.mozilla.org/integration/mozilla-inbound/rev/40dd2c59bd0e

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b978b9e5c2df
https://hg.mozilla.org/mozilla-central/rev/40dd2c59bd0e