Bug 49290 - [Chromium] Fix mouse-up clickCount on the Mac, and in plugins
Summary: [Chromium] Fix mouse-up clickCount on the Mac, and in plugins
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Stuart Morgan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-09 17:21 PST by Stuart Morgan
Modified: 2010-11-17 02:02 PST (History)
2 users (show)

See Also:


Attachments
Fix clickCount for mouse-up (1.35 KB, patch)
2010-11-09 17:21 PST, Stuart Morgan
no flags Details | Formatted Diff | Diff
Fix clickCount for mouse-up (correct patch format) (2.36 KB, patch)
2010-11-10 16:00 PST, Stuart Morgan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stuart Morgan 2010-11-09 17:21:38 PST
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
Comment 1 Stuart Morgan 2010-11-10 16:00:30 PST
Created attachment 73551 [details]
Fix clickCount for mouse-up (correct patch format)

Now with ChangeLog
Comment 2 Dimitri Glazkov (Google) 2010-11-16 09:59:10 PST
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
Comment 3 Stuart Morgan 2010-11-16 11:02:20 PST
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.
Comment 4 Dimitri Glazkov (Google) 2010-11-16 11:08:12 PST
(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.
Comment 5 Dimitri Glazkov (Google) 2010-11-16 11:09:24 PST
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.
Comment 6 WebKit Commit Bot 2010-11-17 02:02:35 PST
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>
Comment 7 WebKit Commit Bot 2010-11-17 02:02:41 PST
All reviewed patches have been landed.  Closing bug.