RESOLVED FIXED 35900
[Chromium] Need to send mouse events to plugin when it has mouse capture
https://bugs.webkit.org/show_bug.cgi?id=35900
Summary [Chromium] Need to send mouse events to plugin when it has mouse capture
John Abd-El-Malek
Reported 2010-03-08 19:25:27 PST
This would allow a plugin to implement scrollbars in its region, for example.
Attachments
Proposed patch (3.76 KB, patch)
2010-03-08 19:32 PST, John Abd-El-Malek
no flags
Added missing file (4.59 KB, patch)
2010-03-08 20:38 PST, John Abd-El-Malek
no flags
Updated patch (4.54 KB, patch)
2010-03-09 14:27 PST, John Abd-El-Malek
fishd: review+
fishd: commit-queue-
John Abd-El-Malek
Comment 1 2010-03-08 19:32:24 PST
Created attachment 50270 [details] Proposed patch I don't like how I can't set m_haveMouseCapture to false in mouseCaptureLost(), any suggestions?
WebKit Review Bot
Comment 2 2010-03-08 20:33:24 PST
John Abd-El-Malek
Comment 3 2010-03-08 20:38:32 PST
Created attachment 50276 [details] Added missing file
Darin Fisher (:fishd, Google)
Comment 4 2010-03-08 22:58:01 PST
What if WebWidget::mouseCaptureLost() is called? See the callsite in chrome/renderer/render_widget.cc.
John Abd-El-Malek
Comment 5 2010-03-09 14:03:29 PST
Right, that's my worry, that the window loses mouse capture without a right mouse button up event. Is it possible though?
John Abd-El-Malek
Comment 6 2010-03-09 14:27:37 PST
Created attachment 50350 [details] Updated patch Per Darin's advice, reversed the order that the browser sends the events (http://codereview.chromium.org/743003). This makes this change cleaner.
Darin Fisher (:fishd, Google)
Comment 7 2010-03-09 16:12:30 PST
Comment on attachment 50350 [details] Updated patch > Index: WebKit/chromium/src/WebViewImpl.cpp ... > + if (m_haveMouseCapture && WebInputEvent::isMouseEventType(inputEvent.type)) { > + Node* node = focusedWebCoreNode(); > + if (node && node->renderer() && node->renderer()->isEmbeddedObject()) { > + AtomicString eventType; > + switch (inputEvent.type) { nit: indentation of AtomicString is off. > void WebViewImpl::mouseCaptureLost() > { > + m_haveMouseCapture = false; > } We may eventually wish to pass this event along to the plugin. Are there any compatibility concerns with enabling this patch for ordinary plugins?
John Abd-El-Malek
Comment 8 2010-03-09 17:39:17 PST
My hunch is that there shouldn't be any issue with passing this to NPAPI plugins. Basic testing with Flash hasn't shown any problems, but pushing this out will tell us conclusively.
John Abd-El-Malek
Comment 9 2010-03-09 17:45:22 PST
Committed r55758
Darin Fisher (:fishd, Google)
Comment 10 2010-03-09 21:08:19 PST
(In reply to comment #8) > My hunch is that there shouldn't be any issue with passing this to NPAPI > plugins. Basic testing with Flash hasn't shown any problems, but pushing this > out will tell us conclusively. OK, sounds good.
Note You need to log in before you can comment on or make changes to this bug.