Bug 49290 – [Chromium] Fix mouse-up clickCount on the Mac, and in plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Stuart Morgan]

Created attachment 73441 [details]
Fix clickCount for mouse-up

This fixes two issues with clickCount on mouseup (for http://crbug.com/62348):
- clickCount was never set for any mouse up event on the Mac during the conversion from native NSEvent objects
- clickCount was not being passed down into WebKit internals for events dispatched by the plugin mouse capture system

[Stuart Morgan]

Created attachment 73551 [details]
Fix clickCount for mouse-up (correct patch format)

Now with ChangeLog

[Dimitri Glazkov (Google)]

Comment on attachment 73551 [details]
Fix clickCount for mouse-up (correct patch format)

Good catch, thanks for the patch. All you need now is add a test here: http://google.com/codesearch/p#OAMlx_jo-ck/src/third_party/WebKit/WebKit/chromium/tests/PopupMenuTest.cpp&exact_package=chromium

[Stuart Morgan]

It's unclear to me how to test this in the current infrastructure.
- The plugin-level is problematic because Mac plugins have two different event models, and only one of them provides clickCount. So, changing the test plugin to log clickCount doesn't seem like an option, since the logging would change based on the event model (which is currently not the case; it's deliberately normalized between the two)
- The factory-level requires an actual NSEvent. The tests that I see fabricate events using one of the WebKit-internal structures, not actual OS events.

[Dimitri Glazkov (Google)]

(In reply to comment #3)
> It's unclear to me how to test this in the current infrastructure.
> - The plugin-level is problematic because Mac plugins have two different event models, and only one of them provides clickCount. So, changing the test plugin to log clickCount doesn't seem like an option, since the logging would change based on the event model (which is currently not the case; it's deliberately normalized between the two)
> - The factory-level requires an actual NSEvent. The tests that I see fabricate events using one of the WebKit-internal structures, not actual OS events.

Ok. Given that this is a pretty clear oversight in a rarely-changing code, I am happy to file a bug on this and r+ the patch.

[Dimitri Glazkov (Google)]

Comment on attachment 73551 [details]
Fix clickCount for mouse-up (correct patch format)

View in context: https://bugs.webkit.org/attachment.cgi?id=73551&action=review

Please file a bug on test infrastructure.

> WebKit/chromium/ChangeLog:13
> +        (WebKit::WebViewImpl::handleInputEvent):

For your future WebKit patches, it's a good form to specify here what changed.

[WebKit Commit Bot]

Comment on attachment 73551 [details]
Fix clickCount for mouse-up (correct patch format)

Clearing flags on attachment: 73551

Committed r72181: <http://trac.webkit.org/changeset/72181>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.