Bug 13274 – REGRESSION: Mouse events are not always sent to the "captured" subframe when dragging

• Component: Forms
• Status: RESOLVED FIXED

[Dex Deacon]

If you click and hold the mouse in a subframe, then drag the mouse out of that subframe, it will stop receiving mousemove events.  Interestingly, if you drag the mouse over a *different* subframe, the original subframe will receive the events.  This is because of some weird logic in EventHandler; passMouseMoveEventToSubframe is called only if the mouse is over a subframe, and the target is then changed to the "captured" subframe's view (m_mouseDownView).

I think the fix is to remember the captured subframe, and call passMouseMoveEventToSubframe if either 1) we have a capture subframe, or 2) the mouse is over a subframe.  m_capturingMouseEventsNode looks like a convenient place to store the captured subframe.

Test case coming shortly.

[Dex Deacon]

Created attachment 13940 [details]
test page

[Dex Deacon]

Created attachment 13941 [details]
subframe for test page

Save this as "subframe.html" in the same place as the above file.

[Dex Deacon]

Created attachment 13950 [details]
possible patch

Here's my attempt at a patch.  It uses the existing m_capturingMouseEvents variable on EventHandler to remember that future events should be routed to a subframe.  The layout tests pass, but I'm not quite sure if it would interfere with other uses of m_capturingMouseEvents (or if it does, maybe it should).

[Dave Hyatt]

Comment on attachment 13950 [details]
possible patch

I don't think this is the right approach.  There are really two kinds of capture: there is capture for a node within a document, and then there is widget-level capture (which can apply to plugins or to frames).

I would argue that it is a platform's responsibility to do widget-level capture (and this capture automatically takes care of the subframe problem, since events just go straight to the correct widget).

[Dave Hyatt]

This problem basically needs to be addressed in such a way that WebKit plugins work too.  I think it's really just a Mac-specific WebKit event routing issue and that the fix will end up being in the Mac WebKit code.

[Dex Deacon]

I think if you do widget-level capture, you'll end up with a weird inconsistency.  Right now, mousemove events are sent to every frame that the mouse is over (for example, in my test page, mouse over the subframe, and both the subframe and toplevel receive mousemove events).  With widget-level capture, only the captured widget will receive events.  So you'll have this situation where the toplevel frame always receives mousemove events, unless a subframe has capture.

Not sure if that really matters, though.

[Dave Hyatt]

I agree that it would be inconsistent, but I think it's a bug that the mousemove goes to the parent at all.

The parent frame should not be getting mousemoves while you are in the child frame in my opinion (and you'll see that it does not in Firefox).

[Dave Hyatt]

We're effectively firing two unique mousemove events and sending them to each frame.  That's not correct and is worthy of a completely separate bug.

[Dave Hyatt]

Looking into this more, this code is a total mess.

The lastMouseMovedFrame (whose purpose is really just to properly send a mouseout) is improperly interacting with the Mac-specific m_mouseDownView.

The m_mouseDownView code is also broken and doesn't even work.

I'm going to look into patching this.

[Dave Hyatt]

Safari 2 behaves correctly regarding capture behavior,  so this is a P1 regression.

[Dave Hyatt]

Note that in Safari 2, the capture mousemoves also go to all frames in the chain (just like non-capturing mousemoves), so Safari 2 is consistent.

[Dave Hyatt]

So I've basically done a complete 180 here and now think your patch is the way to go.  It has the advantage of still working even if a widget toolkit sends things directly to a specific widget.

[Dave Hyatt]

Created attachment 13975 [details]
Build upon previous patch

This builds on the previous patch and also fixes the mouse move problem.

I also tweaked invalidateClick for the case where a widget toolkit does end up going directly to a specific subframe so that m_mousePressed won't get "stuck" but clears immediately as mouse down delivers events to subframes.

So now we basically have three kinds of capture: frame capture, widget capture and node capture.  Frame and node capture are at least using a similar cross-platform mechanism.  

Widget capture is basically buggy on the Mac now, but we've wallpapered over the most obvious bugs by getting frame capture right.  Eventually Mac will start sending events right to subviews, but in the mean time adding this extra frame capture layer seems pretty straightforward and safe.

[Dave Hyatt]

Comment on attachment 13975 [details]
Build upon previous patch

Hmmm still have some issues with this patch.

(1) It's bad to do extra hit testing if you are already capturing.  Should restructure the code to not waste time doing that.

(2) Clearing capturing has to be robust and crawl up the frame tree to future-proof for widget toolkits sending events straight to specific subframes.

[Dave Hyatt]

Fixed in r20761.