This is true for the Qt, Gtk, and Windows implementations. This is the Qt one, the other two are almost the same: bool PluginView::dispatchNPEvent(NPEvent& event) { ... bool accepted = m_plugin->pluginFuncs()->event(m_instance, &event); ... return acccepted; } callers are using it this way: if (!dispatchNPEvent(npEvent)) event->setDefaultHandled(); This works because the plugin function returns with 0 if the plugin handled the event but it's clear that it is messed up. dispatchNPEvent should return with the inverted value and the caller should call setDefaultHandled if the return value is true.
(In reply to comment #0) > This is true for the Qt, Gtk, and Windows implementations. This is the Qt one, the other two are almost the same: Symbian is also suffering from this. According to the test plugin (TestNetscapePlugin) on Mac carbon plugins are also returning with 0 if the event has been handled but I won't fix that case in this patch.
(In reply to comment #1) > (In reply to comment #0) > > This is true for the Qt, Gtk, and Windows implementations. This is the Qt one, the other two are almost the same: > > Symbian is also suffering from this. According to the test plugin (TestNetscapePlugin) on Mac carbon plugins are also returning with 0 if the event has been handled but I won't fix that case in this patch. Forget to mention: Cocoa plugins return with 1 if the event has been handled (according to the test plugin).
Created attachment 98334 [details] Patch
Comment on attachment 98334 [details] Patch It should be possible to write a regression test for this, right?
(In reply to comment #4) > (From update of attachment 98334 [details]) > It should be possible to write a regression test for this, right? I cannot imagine any way to test this. The behavior is the same, it is actually a refactoring to make the code readable. Neither the plugin nor WebCore see any effect of this change.
(In reply to comment #0) > This works because the plugin function returns with 0 if the plugin handled the event but it's clear that it is messed up. http://devedge-temp.mozilla.org/library/manuals/2002/plugin/1.0/npp_api6.html says: If the plug-in handles the event, [NPP_HandleEvent] should return true. If the plug-in ignores the event, [NPP_HandleEvent] returns false. This doesn't seem to match what you're saying.
(In reply to comment #6) > (In reply to comment #0) > > This works because the plugin function returns with 0 if the plugin handled the event but it's clear that it is messed up. > > http://devedge-temp.mozilla.org/library/manuals/2002/plugin/1.0/npp_api6.html says: > > If the plug-in handles the event, [NPP_HandleEvent] should return true. > If the plug-in ignores the event, [NPP_HandleEvent] returns false. > > This doesn't seem to match what you're saying. Unfortunately only Cocoa plugins work that way.
Comment on attachment 98334 [details] Patch Clearing flags on attachment: 98334 Committed r89576: <http://trac.webkit.org/changeset/89576>
All reviewed patches have been landed. Closing bug.
(In reply to comment #7) > > > > If the plug-in handles the event, [NPP_HandleEvent] should return true. > > If the plug-in ignores the event, [NPP_HandleEvent] returns false. > > > > This doesn't seem to match what you're saying. > > Unfortunately only Cocoa plugins work that way. Really? The following seems to CARBON if I'm not mistaken ... http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/mac/PluginViewMac.mm?rev=89576#L602
(In reply to comment #10) > (In reply to comment #7) > > > > > > If the plug-in handles the event, [NPP_HandleEvent] should return true. > > > If the plug-in ignores the event, [NPP_HandleEvent] returns false. > > > > > > This doesn't seem to match what you're saying. > > > > Unfortunately only Cocoa plugins work that way. > > Really? The following seems to CARBON if I'm not mistaken ... > http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/mac/PluginViewMac.mm?rev=89576#L602 Yes it is. And it says: if (!dispatchNPEvent(record)) LOG(Events, "PluginView::handleKeyboardEvent(): Keyboard event type %d not accepted", record.what); 683 else 684 event->setDefaultHandled();
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #7) > > > > > > > > If the plug-in handles the event, [NPP_HandleEvent] should return true. > > > > If the plug-in ignores the event, [NPP_HandleEvent] returns false. > > > > > > > > This doesn't seem to match what you're saying. > > > > > > Unfortunately only Cocoa plugins work that way. > > > > Really? The following seems to CARBON if I'm not mistaken ... > > http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/mac/PluginViewMac.mm?rev=89576#L602 > Again. Last time I was committing the text accidentally, sorry. So, it says: if (!dispatchNPEvent(record)) LOG(Events, "PluginView::handleKeyboardEvent(): Keyboard event type %d not accepted", record.what); else event->setDefaultHandled(); which means the event has been handled if NPP_HandleEvent returned with 0 a.k.a false.