Bug 120055 - [GTK] wmode='transparent' for flash plugin doesn't work
Summary: [GTK] wmode='transparent' for flash plugin doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-20 01:30 PDT by ChangSeok Oh
Modified: 2014-04-27 11:28 PDT (History)
5 users (show)

See Also:


Attachments
TestCase (14.94 KB, application/gzip)
2013-08-20 01:32 PDT, ChangSeok Oh
no flags Details
Patch (2.11 KB, patch)
2013-08-20 02:07 PDT, ChangSeok Oh
andersca: review-
Details | Formatted Diff | Diff
Patch (6.32 KB, patch)
2014-04-22 11:14 PDT, Carlos Garcia Campos
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ChangSeok Oh 2013-08-20 01:30:01 PDT
I'm not sure this issue belongs to webkit or flash plugin.
Comment 1 ChangSeok Oh 2013-08-20 01:32:25 PDT
Created attachment 209166 [details]
TestCase
Comment 2 ChangSeok Oh 2013-08-20 02:07:01 PDT
Created attachment 209169 [details]
Patch
Comment 3 Gustavo Noronha (kov) 2013-08-21 18:42:24 PDT
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?
Comment 4 Anders Carlsson 2013-09-13 09:38:11 PDT
Comment on attachment 209169 [details]
Patch

This should be handled inside NetscapePluginX11, not inside the frame loader client.
Comment 5 Carlos Garcia Campos 2014-04-11 06:27:38 PDT
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.
Comment 6 Carlos Garcia Campos 2014-04-22 11:14:02 PDT
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.
Comment 7 Gustavo Noronha (kov) 2014-04-23 06:43:59 PDT
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?).
Comment 8 Carlos Garcia Campos 2014-04-23 06:48:27 PDT
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.
Comment 9 Carlos Garcia Campos 2014-04-24 09:00:14 PDT
Committed r167759: <http://trac.webkit.org/changeset/167759>
Comment 10 Carlos Garcia Campos 2014-04-24 09:02:20 PDT
(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
Comment 11 Zan Dobersek 2014-04-27 11:28:52 PDT
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