Closed
Bug 774458
Opened 12 years ago
Closed 12 years ago
Make mouse events synthesized from touch events comply with spec (in gonk widget backend and cross-process code)
Categories
(Core :: DOM: Events, defect, P1)
Tracking
()
RESOLVED
FIXED
B2G C2 (20nov-10dec)
People
(Reporter: cjones, Assigned: mwu)
References
Details
(Whiteboard: [LOE:S])
Attachments
(1 file)
(deleted),
patch
|
cjones
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•12 years ago
|
||
This should block basecamp. According to smaug, we're in for a world of incompatibly pain with current event-dispatch behavior.
blocking-basecamp: --- → ?
Reporter | ||
Comment 2•12 years ago
|
||
*incompatibility
Updated•12 years ago
|
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +
Comment 3•12 years ago
|
||
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.
Reporter | ||
Comment 4•12 years ago
|
||
Note, the brokenness originally referred to, and what this bug covers, has nothing to do with the followup hack for cross-process focus.
Updated•12 years ago
|
Assignee: nobody → mwu
Reporter | ||
Comment 5•12 years ago
|
||
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.
Updated•12 years ago
|
Whiteboard: [WebAPI:P0]
Assignee | ||
Comment 6•12 years ago
|
||
(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•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
(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.
Assignee | ||
Updated•12 years ago
|
Whiteboard: [WebAPI:P0] → [WebAPI:P0][LOE:S]
Updated•12 years ago
|
Priority: -- → P1
Whiteboard: [WebAPI:P0][LOE:S] → [LOE:S]
Assignee | ||
Comment 9•12 years ago
|
||
(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•12 years ago
|
||
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.
Assignee | ||
Comment 11•12 years ago
|
||
(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•12 years ago
|
||
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.
Target Milestone: --- → B2G C2 (20nov-10dec)
Comment 13•12 years ago
|
||
No update for over a month. Michael, what needs to happen here?
Assignee | ||
Comment 14•12 years ago
|
||
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.
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #686821 -
Flags: review?(jones.chris.g)
Reporter | ||
Comment 16•12 years ago
|
||
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.
Attachment #686821 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 17•12 years ago
|
||
(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.
Reporter | ||
Comment 18•12 years ago
|
||
Switching to touch events where supported in the master process will be a small perf win too.
Assignee | ||
Comment 19•12 years ago
|
||
Updated•12 years ago
|
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
Comment 20•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 21•12 years ago
|
||
Comment 22•12 years ago
|
||
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?
Assignee | ||
Comment 23•12 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•