[GTK] wmode='transparent' for flash plugin doesn't work

• Component: WebKitGTK
• Status: RESOLVED FIXED

[ChangSeok Oh]

I'm not sure this issue belongs to webkit or flash plugin.

[ChangSeok Oh]

Created attachment 209166 [details]
TestCase

[ChangSeok Oh]

Created attachment 209169 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 209169 [details]
Patch

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

> Source/WebKit2/ChangeLog:10
> +        wmode='transparent' seems not to be supported properly on X11. According
> +        to the above comment, we don't support transparent and windowed mode, but there is
> +        no workaround for wmode=transparent. As my understanding, this is because the visual
> +        of the window provied to flash plugin has only 24 depth bits which doesn't include alpha bits.

I don't understand from this description what change in behaviour your change causes and why it helps with the issue, could you improve the changelog to cover those things?

[Anders Carlsson]

Comment on attachment 209169 [details]
Patch

This should be handled inside NetscapePluginX11, not inside the frame loader client.

[Carlos Garcia Campos]

The patch doesn't solve the issue for me, I see the background yellow instead of transparent. I also think this should be converted to quirks and moved to NetscapePlugin::initialize() which is where the parameters are received and passed to the plugin. 

I think the best solution would be to properly implement transparency, but since the flash plugin requires the passed visual to be the same as the system visual, and system visual normally has a 24 depth, we would need to use opaque surfaces and fake the transparency like we did in WebKit1. The problem is that now I think we don't have access to the background from the plugin paint function, so we can't fill the plugin xlib surface with the background before sending the expose event.

[Carlos Garcia Campos]

Created attachment 229899 [details]
Patch

Even when this doesn't fix the issue, it's indeed the intended behaviour according to the comment, so I've added the transparent case and made it a new plugin quirk for it.

[Gustavo Noronha (kov)]

Comment on attachment 229899 [details]
Patch

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

> Source/WebKit2/Shared/Plugins/PluginQuirks.h:94
> +        // Some ports don't support windowed plugins.
> +        ForceFlashWindowlessMode,
> +

Should this be under #if PLUGIN_ARCHITECTURE(X11)?

> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:616
> +#endif

You're no longer injecting windowedPlugin=false for application/x-webkit-test-netscape, I assume that's not required (anymore?).

[Carlos Garcia Campos]

Comment on attachment 229899 [details]
Patch

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

Thanks for the review.

>> Source/WebKit2/Shared/Plugins/PluginQuirks.h:94
>> +
> 
> Should this be under #if PLUGIN_ARCHITECTURE(X11)?

It is

>> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:616
>> +#endif
> 
> You're no longer injecting windowedPlugin=false for application/x-webkit-test-netscape, I assume that's not required (anymore?).

No, I forgot it, but I wonder why that is needed. If EFL doesn't support windowed plugins, layout tests using windowed plugins should probably be skipped for EFL port.

[Carlos Garcia Campos]

Committed r167759: <http://trac.webkit.org/changeset/167759>

[Carlos Garcia Campos]

(In reply to comment #8)
> (From update of attachment 229899 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229899&action=review
> 
> Thanks for the review.
> 
> >> Source/WebKit2/Shared/Plugins/PluginQuirks.h:94
> >> +
> > 
> > Should this be under #if PLUGIN_ARCHITECTURE(X11)?
> 
> It is
> 
> >> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:616
> >> +#endif
> > 
> > You're no longer injecting windowedPlugin=false for application/x-webkit-test-netscape, I assume that's not required (anymore?).
> 
> No, I forgot it, but I wonder why that is needed. If EFL doesn't support windowed plugins, layout tests using windowed plugins should probably be skipped for EFL port.

Landed the patch, I'll keep watching the bots, if the test plugin hack is still needed I'll fix it in a follow up patch

[Zan Dobersek]

This seems to have made some tests crashing on the 64-bit debug builder, leading to early exits. Carlos, could you please take a look?
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/41774