Closed
Bug 742445
Opened 13 years ago
Closed 12 years ago
Mouse DOM events not being detected
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: bbondy, Assigned: jimm)
References
Details
(Whiteboard: [completed-elm])
Attachments
(5 files)
If you create an HTML page with javascript events for onmousemove, onmouseover, and onmouseout with a simple alert, the alert will be shown on a desktop browser but not in the Metro browser.
Alerts work fine but something is not hooked up yet because the events are not fired.
Reporter | ||
Comment 1•12 years ago
|
||
Re-reading the above it sounds a little confusing. What I meant is that events are not firing for things like onmouseover inside the metro browser.
Comment 2•12 years ago
|
||
We could add a message that gets sent to browser content and tells content.js to synthesize mousemove/mouseover/mouseout whenever there are corresponding events in the chrome layer. It'll maintain compatibility with e10s, but I'm not sure how performant it'll be.
We could also refactor the browser XBL to be purely in-process and not use messages. I'm guessing the former will get us where we need to be faster, though?
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Jonathan Wilde [:jwilde] from comment #2)
> We could add a message that gets sent to browser content and tells
> content.js to synthesize mousemove/mouseover/mouseout whenever there are
> corresponding events in the chrome layer. It'll maintain compatibility with
> e10s, but I'm not sure how performant it'll be.
>
> We could also refactor the browser XBL to be purely in-process and not use
> messages. I'm guessing the former will get us where we need to be faster,
> though?
Forwarding to start off if it's less work is probably the right approach at first. We can always rip out the messaging later if we find it's dragging things down.
Comment 4•12 years ago
|
||
DOM touch events are already forwarded via async messaging with the Browser:MouseMove, Browser:MouseOver, and Browser:MouseOut messages. content.js automatically converts the messages into touch events and occasionally mouse events.
What I mean by "occasionally":
TapOver in chrome gets sent as Browser:MouseOver to content, which then gets dispatched as a mousemove event on the appropriate element on the page. With the mousemove event, the element then generates appropriate mouseover and mouseout events.
However, most mouse/touch events are surfaced as touch events in content.
Comment 5•12 years ago
|
||
I was thinking through this. Since having both keyboard *and* mouse events being tracked at once could cause complexity of all of this to go up through the roof, I've been doing some thinking about this.
(A) content.js should have as much 1:1 correspondence between message names and DOM event names as is possible. Given that we'll potentially have both touch and mouse events in the same session (which wasn't the case for Fennec), we need to distinguish between the two. I'd like to propose that there should be separate messages for mouse and touch events.
(B) Deciding whether to send touch, mouse events, or both, should happen at the chrome level. This will reduce the amount of processing that needs to happen individually in each tab and eliminates the need for sending events about pointer precision to content.
Updated•12 years ago
|
Assignee: nobody → jonathan
Status: NEW → ASSIGNED
Comment 6•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #3)
> (In reply to Jonathan Wilde [:jwilde] from comment #2)
> > We could add a message that gets sent to browser content and tells
> > content.js to synthesize mousemove/mouseover/mouseout whenever there are
> > corresponding events in the chrome layer. It'll maintain compatibility with
> > e10s, but I'm not sure how performant it'll be.
> >
> > We could also refactor the browser XBL to be purely in-process and not use
> > messages. I'm guessing the former will get us where we need to be faster,
> > though?
>
> Forwarding to start off if it's less work is probably the right approach at
> first. We can always rip out the messaging later if we find it's dragging
> things down.
I've been tinkering with this over the past two days. The async messaging is fine for mouseup/mousedown, but things get bogged down somewhat with mousemove events:
- The :hover states of elements under the cursor lag.
- The cursor rapidly switches between default and pointer as you move a mouse across a link.
- If you move the mouse over an element that's CSS transitioning, it'll flicker/lag.
I'm guessing that all of the above issues are actually surmountable, given that they don't appear while the user is selecting text.
Comment 7•12 years ago
|
||
I'm also running into an issue where it seems as though mouseup/click/dblclick events won't be fired at all from the chrome document if mouseup and mousedown are synthesized asynchronously using sendMouseEventToWindow(). mouseup works again if the window is unfocused and then focused.
It's also possibly a focus-related issue in higher level code.
Assignee | ||
Comment 8•12 years ago
|
||
The flicker may be something we can address down in widget. Platform calling SetCursor on the widget repeatedly might cause this. (It shouldn't but I've seen behavior like this before.) If you can post your wip we can investigate what's going on in the backend.
Assignee | ||
Comment 9•12 years ago
|
||
As far as lag goes, this is something I've been concerned about with the async messaging. There's event loop overhead there. Mouse moves generate many backend events. If we have to go with more direct messaging to get in-process working right, we should. We can investigate async event issues if and when we decide to flip back to out-of-process content.
Comment 10•12 years ago
|
||
I'm really not sure what's going on here. After one click in the content area, the mouseup events cease to fully fire (for both the content window and chrome window) until the focus is somehow changed.
It appears that mouseup will cease to fully fire during the next click if a mousedown is fired without synchronously firing a mouseup at the same time from sendMouseEventToWindow.
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to Jonathan Wilde [:jwilde] from comment #10)
> Created attachment 645947 [details] [diff] [review]
> wip
I took this for a spin looking for cursor flicker problems but didn't see any cursor changes. Maybe this wasn't the right patch for that testing?
Comment 12•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #11)
> (In reply to Jonathan Wilde [:jwilde] from comment #10)
> > Created attachment 645947 [details] [diff] [review]
> > wip
>
> I took this for a spin looking for cursor flicker problems but didn't see
> any cursor changes. Maybe this wasn't the right patch for that testing?
Thanks for testing this out. I forgot to merge in the changes specified over in https://bugzilla.mozilla.org/show_bug.cgi?id=775714#c3 in this patch, so the cursor flicker issues only show up in -metrodesktop.
----
I also recently pinged Fennec folks and it sounds like the JS-backed touch events and panning from Fennec-XUL are not in line with what b2g and and Fennec-native are moving towards.
Scrolling management is getting moved from top-level chrome UI code into Gecko (bug 745136) and it's looking like everything's going to center in and around AsyncPanZoomController (bug 775437). From the tracking bug, it looks like there's lots of work still to go on that, though.
Would it be possible to use AsyncPanZoomController for Metro? Or is it still too early-stage for that?
---
At any rate, it's sounding like the current approach for pointer management in Fennec-XUL is likely not how pointer management will end up working in e10s over the long term, since the general trend is for panning/zooming, touch events, etc. to be moved into Gecko itself.
At this point, I'm in favor of stripping out the layers around <browser> and having input get sent directly there like on desktop.
Comment 13•12 years ago
|
||
Did some more reading: it sounds like once bug 745071 lands, if we set layers.async-pan-zoom.enabled to true, we'll get panning and zooming for free.
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Jonathan Wilde [:jwilde] from comment #13)
> Did some more reading: it sounds like once bug 745071 lands, if we set
> layers.async-pan-zoom.enabled to true, we'll get panning and zooming for
> free.
I don't think so, that looks to be related to off main thread compositing which currently only works on b2g w/oopc.
Assignee | ||
Comment 15•12 years ago
|
||
Would you like to hand this off Johnathan? Unless you really want to tackle overhauling our event code maybe I or bbondy can pick this work up.
Assignee | ||
Comment 16•12 years ago
|
||
This is pretty messed up. I think we can clean this up though without breaking frame message manager forwarding we might want down the road.
Assignee | ||
Comment 17•12 years ago
|
||
Same trace but with a stopPropagation in input.js during the event capturing phase. However we still get event loop spins when we call into the frame message manager, which we want to avoid for in process content.
Assignee | ||
Comment 18•12 years ago
|
||
For in process content the message manager has the unfortunate habit of relaying delayed input events. For oopc you don't want content blocking chrome, but for in process ipc delivers events on the ui thread, so we gain nothing with the added negative of out of sync input.
Note that browser.messageManager doesn't support a sendSyncMessage currently.
Assignee | ||
Comment 19•12 years ago
|
||
Here's the crux of the delay - for in process messaging, we fire a runnable, which on Windows posts a message to the bottom of the ui thread's message queue. Most of the time this places it below other incoming input events.
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsFrameLoader.cpp#2194
Assignee | ||
Comment 20•12 years ago
|
||
The same sequence with a hack in snFrameLoader that calls Run on the tunnable directly rather than dispatching it.
To fix this, I don't think we want to completely toss out the use of message manager since we hope to get back to out of process content at some point. So maybe the answer here is to encapsulate our message sending / receiving such that we only use the msg manager when we're running out of process while also keeping our content/chrome message boundary intact.
Comment 21•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #15)
> Would you like to hand this off Johnathan? Unless you really want to tackle
> overhauling our event code maybe I or bbondy can pick this work up.
At this point, handing off would be a good idea.
(In reply to Jim Mathies [:jimm] from comment #20)
> So maybe the answer here is to encapsulate our message sending / receiving
> such that we only use the msg manager when we're running out of process
> while also keeping our content/chrome message boundary intact.
Also, sounds good.
Updated•12 years ago
|
Assignee: jonathan → nobody
Status: ASSIGNED → NEW
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → jmathies
Assignee | ||
Comment 22•12 years ago
|
||
(In reply to Jonathan Wilde [:jwilde] from comment #5)
> I was thinking through this. Since having both keyboard *and* mouse events
> being tracked at once could cause complexity of all of this to go up through
> the roof, I've been doing some thinking about this.
>
> (A) content.js should have as much 1:1 correspondence between message names
> and DOM event names as is possible. Given that we'll potentially have both
> touch and mouse events in the same session (which wasn't the case for
> Fennec), we need to distinguish between the two. I'd like to propose that
> there should be separate messages for mouse and touch events.
>
> (B) Deciding whether to send touch, mouse events, or both, should happen at
> the chrome level. This will reduce the amount of processing that needs to
> happen individually in each tab and eliminates the need for sending events
> about pointer precision to content.
So the way I've got this working currently (if you set 'metro.debug.treatmouseastouch' to false) is that precise mouse events are delivered normally to the browser. Touch events continue to simulate mouse events in content.js. This isn't exactly the same as splitting the two classes up and forwarding both. It just seemed simpler for now to let the browser do it's own thing with mouse input. We can worry about how this will work with remote content later.
Assignee | ||
Comment 23•12 years ago
|
||
Also the following translated events are now sent sync vs. async to address the bad timing inherent in message manager for local content:
"Browser:MouseDown"
"Browser:MouseOver"
"Browser:MouseUp"
"Browser:MouseClick"
"Browser:MouseCancel"
We can add to this list very easily as issues arise. However we should try to keep using message manager whenever possible.
Assignee | ||
Comment 24•12 years ago
|
||
For all intensive purposes this is now fixed on elm. I'll be enabling metro.debug.treatmouseastouch by default as soon as I work out the few remaining issues with precise input.
Whiteboard: completed-elm
Reporter | ||
Comment 25•12 years ago
|
||
awesome!
Assignee | ||
Updated•12 years ago
|
Whiteboard: completed-elm → [completed-elm]
Assignee | ||
Comment 26•12 years ago
|
||
Dead bug.
Updated•10 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•