[X11][WebKit2][Plugins] wrong changing return-value for DefaultHandled

[Kwangmin Bang]

Do not change return-value returned from Plugin by NPP_HandleEvent().
only x11 port is changing this value.
If NPP_HandleEvent return 1, webkit should call setDefaultHandled(). it means plugin handled this event.
If returned 0, webkit should NOT call setDefaulthandled(). it means plugin ignore the event or want to be called defaultHandler.
please refer other platform codes or codes in Webcore/plugins. you can verify this bug on Youtube Video (Flash) through MouseWheelEvent.

[Kwangmin Bang]

Created attachment 135489 [details]
[WebKit2][Plugins] wrong changing return-value for DefaultHandled

upload patch file.

[Simon Hausmann]

Comment on attachment 135489 [details]
[WebKit2][Plugins] wrong changing return-value for DefaultHandled

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

> Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:-447
> -    return !NPP_HandleEvent(&xEvent);

In WebKit1 the code (PluginViewQt.cpp at least) reads like this:

bool dispatchNPEvent(...) {
    ...
    bool accepted = !m_plugin->pluginFuncs()->event(m_instance, &event);
    ...
    return accepted;
}

and then the caller uses

if (dispatchNPEvent(...))
    event->setDefaultHandled();


It appears the code in WebKit2 is similar, where WebProcess/Plugins/PluginView.cpp uses a pattern like this:

    bool didHandleEvent = false;
    ...
    didHandleEvent = m_plugin->handleMouseEvent(...);
    ...
    if (didHandleEvent)
        event->setDefaultHandled();

The return value of platformHandleMouseEvent seems to be passed through and with using !NPP_HandleEvent(...)
I see the same transformation / interpretation of the return value from the plugin.

Either I am missing something, the code in WebKit1 is wrong, or your patch is incorrect :). In order to get this logic correct I think it would be worthwhile to have a layout tests for this, especially since we have the infrastructure in place with a test netscape plugin.

[Kwangmin Bang]

By your comment, i verified that return-value is changed by "!" operator in gtk/qt port.

However, Mozilla NPAPI explains like below.

https://developer.mozilla.org/en/NPP_HandleEvent
If the plug-in handles the event, the function should return true.
If the plug-in ignores the event, the function returns false.

In other words, If plugin return true, webkit should call setDefaultHandled();

By NPAPI guide, I think my patch is right.
And, mac port of webkit2 also does not change that.

What's your opinion ?

[Anders Carlsson]

Comment on attachment 135489 [details]
[WebKit2][Plugins] wrong changing return-value for DefaultHandled

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.

[Alexey Proskuryakov]

Mass closing plug-in bugs, as plug-in support has been removed from WebKit.

Please comment and/or reopen if this still affects WebKit in some way.