Str: - Go to http://maps.google.com/ and start a streetview by dragging the little orange man into the map - Double click on the white rounded geometry that appears around you cursor Result: after the second mouseup the street sticks to your pointer. Regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0099558dc16f&tochange=73e08f744e9a

If you double click in the circle you'll notice that your pointer will start dragging the picture after zooming in. Reproduced with Namoroko.

There's a lot of changes to how we pass mouse events around in that range, though I can't pinpoint a likely suspect. cc'ing Enn, Josh, Roc and Olli who can hopefully narrow things down. I don't think this blocks.

Is streetview using some plugin? Flash? If that is the case, then bug 506304 looks suspicious. Actually, that is pretty much the only change to mouse event handling in that regression range.

It's not bug 506304, I tried backing that out and the bug remains.

Oops, I backed out the wrong part. It's the "test code" part of bug 506304; backing it out fixes the bug.

The strange thing is, there's only one hunk from that patch that actually runs on Windows: nsPluginEvent * pPluginEvent = (nsPluginEvent *)anEvent.nativeMsg; // we can get synthetic events from the nsEventStateManager... these // have no nativeMsg nsPluginEvent pluginEvent; if (anEvent.eventStructType == NS_MOUSE_EVENT) { - // XXX we could synthesize Windows mouse events here for our - // synthetic mouse events (i.e. !pPluginEvent) + if (!pPluginEvent) { ... It only runs when anEvent.nativeMsg is null. That seems strange...

OK, here's what happens for the second mouse-down of the double-click: -- Windows sends WM_LBUTTONDBLCLK -- We dispatch an nsMouseEvent NS_MOUSE_BUTTON_DOWN with clickCount 2 -- This reaches nsPluginInstanceOwner::ProcessEvent and we fire the native event at it (which is still a WM_LBUTTONDBLCLK) Then for the second mouse-up of the double-click: -- Windows sends WM_LBUTTONUP -- We dispatch an nsMouseEvent NS_MOUSE_BUTTON_UP with clickCount 2 -- This reaches nsEventStateManager::CheckForAndDispatchClick, which synthesizes a NS_MOUSE_DOUBLECLICK (since our double-click events fire on mouse-up to be consistent with single-clicks, which makes sense) -- That synthetic double-click reaches nsPluginInstanceOwner::ProcessEvent and, given the patch in bug 506304, we synthesize a native event, another WM_LBUTTONDBLCLK! -- Presumably the plugin thinks the mouse is now down again which is why it starts tracking the mouse movement. The solution is to not synthesize WM_*BUTTONDBLCLK on NS_MOUSE_DOUBLECLICK, but instead send it instead of WM_*MOUSEDOWN when the clickCount is 2.

As described above.

Comment on attachment 420031 [details] [diff] [review] fix Looks good to me although I'm not a peer in layout.

Comment on attachment 420031 [details] [diff] [review] fix a192=beltzner

To write tests for this bug, I need to extend the getLastMouse* functions of the test plugin.

These are the actual tests. They're Windows-only. Basically we dispatch a driver-level sequence of mouse-down, mouse-up, mouse-down, mouse-up --- once for each of the three buttons --- and check that a) it generates the right DOM events and b) the windowless plugin received the expected WM_ events. Then we synthesize the same series of DOM events, at the DOM event level this time, and verify that the windowless plugin received the expected WM_ events (which we synthesized, this time).

This may have caused a regression, see bug 537269

Comment on attachment 420031 [details] [diff] [review] fix >diff --git a/layout/generic/nsObjectFrame.cpp b/layout/generic/nsObjectFrame.cpp >--- a/layout/generic/nsObjectFrame.cpp >+++ b/layout/generic/nsObjectFrame.cpp >@@ -4349,31 +4349,34 @@ nsEventStatus nsPluginInstanceOwner::Pro > const nsMouseEvent* mouseEvent = static_cast<const nsMouseEvent*>(&anEvent); > switch (anEvent.message) { > case NS_MOUSE_MOVE: > pluginEvent.event = WM_MOUSEMOVE; > break; > case NS_MOUSE_BUTTON_DOWN: { > static const int downMsgs[] = > { WM_LBUTTONDOWN, WM_MBUTTONDOWN, WM_RBUTTONDOWN }; >- pluginEvent.event = downMsgs[mouseEvent->button]; >+ static const int dblClickMsgs[] = >+ { WM_LBUTTONDBLCLK, WM_MBUTTONDBLCLK, WM_RBUTTONDBLCLK }; >+ if (mouseEvent->clickCount == 2) { >+ pluginEvent.event = dblClickMsgs[mouseEvent->button]; Are we quite sure that the maximum value of clickCount here can be 2? Reading the code around <http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#3290>, it appears that the clickCount can exceed 2 if you continue clicking without moving your mouse. Shouldn't the check here be >= 2?

I don't think this could have caused bug 537269, since this patch could not have affected Mac. I don't think we should send WM_*DBLCLK events for triple clicks etc. Windows doesn't.