774458 - Make mouse events synthesized from touch events comply with spec (in gonk widget backend and cross-process code)

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P1)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

It came out in bug 774139 that they're currently broken.  We need to fix this.

I don't 100% understand what we need to do, but I think it involved queueing touch events in nsBaseWidget, and then synthesizing mouse events at the end of an event sequence if the touch events weren't consumed.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This should block basecamp.  According to smaug, we're in for a world of incompatibly pain with current event-dispatch behavior.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

*incompatibility

Comment 3

[Wesley Johnston (:wesj)]

The bug I originally brought this up for has to do with matching behaviour of other mobile browser (although the spec does address it in its default action section). Namely, I don't think any mobile browser except b2g current dispatches mouse events (specifically mousedown and mouseup) while you are dragging your finger around on a screen. They only dispatch them on taps.

They only dispatch mouse events when a gesture is complete and then, if they've decided it is a tap, dispatch mousedown/mouseup/click. Sending mouseevents in other cases could lead to developers writing webapps that don't work in other mobile browsers. It could also lead to apps receiving combinations of input they don't normally expect (i.e. assuming that no mouse events will fire if they receive a touchmove?)

mousemove is even less strictly defined. fennec only sends it before its going to fire a click, but you can fire it earlier in some cases to make things like :hover menus easier to use.

I'm not aware off the top of my head of any sites that break in these situations either though (and it may give you things like drag events for free?)

mwu mentioned on IRC that b2g also isn't correctly handling preventDefault right now. We should file a separate bug to fix that.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Note, the brokenness originally referred to, and what this bug covers, has nothing to do with the followup hack for cross-process focus.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Note, the blocking part of this work is spec compliance in content processes, since that's where all interesting content will run.  Spec compliance for the master process is nice to have except where it conflicts with content processes.

Comment 6

[Michael Wu [:mwu]]

(In reply to Wesley Johnston (:wesj) from comment #3)
> The bug I originally brought this up for has to do with matching behaviour
> of other mobile browser (although the spec does address it in its default
> action section). Namely, I don't think any mobile browser except b2g current
> dispatches mouse events (specifically mousedown and mouseup) while you are
> dragging your finger around on a screen. They only dispatch them on taps.
> 

How does fennec avoid this, then? AFAICT, the android and gonk widget backend touch event impls are very similar in when they dispatch mouse events. I'm not sure how fennec could do something differently. The code at http://hg.mozilla.org/mozilla-central/file/a41731220fec/widget/android/nsWindow.cpp#l846 indicates that mouse up/down/move events are fired immediately after the corresponding touch up/down/move events if preventDefault is not called.

Comment 7

[Wesley Johnston (:wesj)]

We have a build time config option to do nothing:

http://hg.mozilla.org/mozilla-central/file/a41731220fec/widget/android/nsWindow.cpp#l1290

The hover events there aren't sent unless the device is in ExploreByTouch mode (accessibility). We currently send mouse events from browser.js:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3519

although I want to move that into widget now that bug 780847 has landed.

Comment 8

[Michael Wu [:mwu]]

(In reply to Wesley Johnston (:wesj) from comment #7)
> We have a build time config option to do nothing:
> 
> http://hg.mozilla.org/mozilla-central/file/a41731220fec/widget/android/
> nsWindow.cpp#l1290
> 
> The hover events there aren't sent unless the device is in ExploreByTouch
> mode (accessibility). We currently send mouse events from browser.js:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#3519
> 
> although I want to move that into widget now that bug 780847 has landed.

Ah perfect. Can you make this bug depend on that once it's filed? I can then make a gonk port of it. Alternately you can port it and morph this bug to cover both widgets.

Comment 9

[Michael Wu [:mwu]]

(In reply to Wesley Johnston (:wesj) from comment #7)
> We have a build time config option to do nothing:
> 
> http://hg.mozilla.org/mozilla-central/file/a41731220fec/widget/android/
> nsWindow.cpp#l1290
> 
> The hover events there aren't sent unless the device is in ExploreByTouch
> mode (accessibility). We currently send mouse events from browser.js:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#3519
> 
> although I want to move that into widget now that bug 780847 has landed.

Wes, bug 788073 didn't move the mouse clicking logic into the widget side. Are you still planning to do so in another bug?

Comment 10

[Wesley Johnston (:wesj)]

I filed bug 803313. There are a few extra pieces there that may make it more difficult (i.e. making sure the element that's highlighted is the one that gets the dispatched event, I think we'll need to make sure we dispatch the mouse events with the same coordinates as the touchdown event we used to find the highlight element).

I'm also not familiar with B2G's gesture detection code, but it would be nice to make sure we're all hitting the same code paths there. Can you point me to it mwu? Otherwise I think we'll just be sending a message from Java to widget instead of Java to JS, which really doesn't change much.

Comment 11

[Michael Wu [:mwu]]

(In reply to Wesley Johnston (:wesj) from comment #10)
> I'm also not familiar with B2G's gesture detection code, but it would be
> nice to make sure we're all hitting the same code paths there. Can you point
> me to it mwu? Otherwise I think we'll just be sending a message from Java to
> widget instead of Java to JS, which really doesn't change much.

We don't have gesture detection on the widget side - any gesture detection is done by gaia on the frontend.

Comment 12

[Alex Keybl [:akeybl]]

This bug has been called out as likely having risk to non-B2G platforms. Given that, marking as P1, and moving into the C2 milestone. We should prioritize this landing to mozilla-beta as soon as possible, to prevent late-breaking regressions to other platforms.

Comment 13

[Dietrich Ayala (:dietrich)]

No update for over a month. Michael, what needs to happen here?

Comment 14

[Michael Wu [:mwu]]

It turns out that for content that get async pan zoom, we're actually pretty close to the right behavior already. Async pan also has a gesture detector which does the right thing, except for when double tapping.

Comment 15

[Michael Wu [:mwu]]

Attachment: Only send clicks on confirmed single taps

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 686821 [details] [diff] [review]
Only send clicks on confirmed single taps

This looks correct, but it's not going to affect bug 811198, which seems to be caused by non-conformance in the sync scrolling world.

Comment 17

[Michael Wu [:mwu]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #16)
> Comment on attachment 686821 [details] [diff] [review]
> Only send clicks on confirmed single taps
> 
> This looks correct, but it's not going to affect bug 811198, which seems to
> be caused by non-conformance in the sync scrolling world.

Yeah.. I'll use bug 811198 to tackle this issue in the sync scrolling world. Parts of gaia (lockscreen, at least) depend on the current behavior. I think we can disable all mouse events except for the ones generated on a tap to get this complaint in the sync world. We'd just have to fix gaia.

Comment 18

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Switching to touch events where supported in the master process will be a small perf win too.

Comment 19

[Michael Wu [:mwu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ccaeeae85df

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1ccaeeae85df

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fc1cb75a0e84
https://hg.mozilla.org/releases/mozilla-beta/rev/2e4c8d76f404

Comment 22

[Wesley Johnston (:wesj)]

From talking to some Gaia devs, I don't think this fixed the bug. We're still sending mouse events from:

http://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsAppShell.cpp#438

Android does something similar to support XUL Fennec (which required mouse events), but uses a flag to turn it off in Native Fennec:

http://mxr.mozilla.org/mozilla-central/source/widget/android/nsWindow.cpp#1173

The more correct fix is likely to look at the toolType of the MotionEvent sent from Android:
http://developer.android.com/reference/android/view/MotionEvent.html#getToolType%28int%29
If its a mouse send the mouse events, otherwise send the touch. The gesture detector in the ipc code can handle the generated events necessary for compatibility with desktop.

Fixing this will likely break a bunch of Gaia apps that currently depend on mouse events (and perhaps desktop Gaia?) Am I reading this right mwu?

Comment 23

[Michael Wu [:mwu]]

(In reply to Wesley Johnston (:wesj) from comment #22)
> Fixing this will likely break a bunch of Gaia apps that currently depend on
> mouse events (and perhaps desktop Gaia?) Am I reading this right mwu?

This bug wasn't entirely fixed - I fixed one aspect of it which primarily affects pages loaded in the browser. I was planning to fix the other aspect in bug 811198 but it turns out that that bug was a useragent detection issue and fixing it would actually make things worse for that particular game.

This isn't Android BTW - we don't have anything that's implemented in Java. We have part of their input handling code which may let us detect mouse vs. touch events. Mouse events aren't actually supported except for testing purposes on pandaboard so it doesn't matter.

We'll need a new bug to fix this issue in non-async scroll content but the risk vs. reward there doesn't seem to be right. A lot of gaia would break. I looked at just killing mouse events on webapps, but there's also no evidence so far that adding mouse events would improve compatibility. It seems like webapps largely just check useragents to determine whether touch events can be used.