WebKit/chromium/src/gtk/WebInputEventFactory.cpp doesn't set clickCount for mouse up events. We probably want to have similar behavior as the win implementation.
I will upload a patch soon.
Created attachment 90248 [details] Set click count for mouse up events, using similar algorithm as win implementation.
The reason why we need such a fix is that the Pepper API passes this information to plugins, and some consumers (e.g., Flash) need it to detect double-clicks.
Comment on attachment 90248 [details] Set click count for mouse up events, using similar algorithm as win implementation. View in context: https://bugs.webkit.org/attachment.cgi?id=90248&action=review > Source/WebKit/chromium/ChangeLog:6 > + [chromium] linux chromium doesn't set click count for mouse up events. > + https://bugs.webkit.org/show_bug.cgi?id=58921 Please explain what this is breaking (ppapi, flash, whatever). > Source/WebKit/chromium/src/gtk/WebInputEventFactory.cpp:492 > + if (shouldForgetPreviousClick(event->window, event->time, event->x, event->y)) > + resetClickCountState(); Why do we need to reset here?
Created attachment 90256 [details] Revise the ChangeLog
Comment on attachment 90248 [details] Set click count for mouse up events, using similar algorithm as win implementation. View in context: https://bugs.webkit.org/attachment.cgi?id=90248&action=review >> Source/WebKit/chromium/ChangeLog:6 >> + https://bugs.webkit.org/show_bug.cgi?id=58921 > > Please explain what this is breaking (ppapi, flash, whatever). Done. >> Source/WebKit/chromium/src/gtk/WebInputEventFactory.cpp:492 >> + resetClickCountState(); > > Why do we need to reset here? I am not sure whether this is very reasonable, but I tried to follow the behavior of the win implementation.
Comment on attachment 90256 [details] Revise the ChangeLog I think this patch is correct, but it would be nice to add a layout test for this. The benefit of a layout test is that it would ensure that we get consistent results across platforms. Specifically, you can use window.eventSender.leapForward and mouseDown/Up to simulate mouse events. You might need to add eventSender.clickCount() for this. This could be a follow up patch.
(+avi since I think he touched this code a bunch in the past)
Hi, Tony. Thanks for your review! I will work on a layout test in a follow up patch. Do you mean I should wait for avi's feedback before committing the patch?
(In reply to comment #9) > Do you mean I should wait for avi's feedback before committing the patch? I think it's fine to commit now. We can revert if avi has additional feedback. Please make sure the layout tests pass before committing. Thanks!
(In reply to comment #9) > Do you mean I should wait for avi's feedback before committing the patch? The change looks good to me.
Hi, Tony. Would you please help to get this patch committed? Thanks!
Comment on attachment 90256 [details] Revise the ChangeLog Clearing flags on attachment: 90256 Committed r84427: <http://trac.webkit.org/changeset/84427>
All reviewed patches have been landed. Closing bug.