Bug 43484 – [Qt] Setting wmode to "opaque" is not necessary for Symbian

• Component: Plug-ins
• Status: RESOLVED FIXED

[Hui Huang]

In Webkit/qt/WebCoreSupport/FrameLoaderClientQt.cpp FrameLoaderClientQt::createPlugin:
        if (mimeType == "application/x-shockwave-flash") {
1463	            QWebPageClient* client = m_webFrame->page()->d->client;
1464	            if (!client || !qobject_cast<QWidget*>(client->pluginParent())) {
1465	                // inject wmode=opaque when there is no client or the client is not a QWebView
1466	                size_t wmodeIndex = params.find("wmode");
1467	                if (wmodeIndex == -1) {
1468	                    params.append("wmode");
1469	                    values.append("opaque");
1470	                } else
1471	                    values[wmodeIndex] = "opaque";
1472	            }

wmode parameter is always changed to opaque. the original value should be preserved for Flash Plugins that supports wmode=window as default.

[Hui Huang]

Created attachment 63594 [details]
Fix Bug 43484

wmode is changed to windowless for Flash Plugins. Preserve the original wmode for Flash Plugins that prefer to support window and windowless transparent modes.

[Eric Seidel (no email)]

I'm confused.  Are you editing the flash code?

Why can't flash just reach out and read from the DOM itself it if needs to find the un-altered wmode parameter anyway?

[Hui Huang]

Eric,
Thanks for the comments. Are you suggesting that Symbian Flash Player use NPN_GetValue(NPNVPluginElementNPObject) to get the DOM element that loaded the plugin and then use NPN_GetProperty to get the unaltered wmode? Thanks

[Eric Seidel (no email)]

How does the symbian flash player work with other engines (like Gecko)?  Do they also ahve this "origwmode" parameter?

[Hui Huang]

I see your point, but the other browser Symbian Flash Player works with doesn't modify wmode parameter to be "opaque" all the time. What about "transparent" and "window" which is supposed to be the default mode? Would be nice if Webkit could pass wmode to the plugin and let it decide which mode it runs in.

[Eric Seidel (no email)]

Comment on attachment 63594 [details]
Fix Bug 43484

It sounds like symbian builds of WebKit want to #ifdef out the section of code which rewrites the param value then, instead of adding a new param.

[Hui Huang]

I think you are right. I will ifdef out the code that rewrites the parameter value for Symbian, retest with Symbian Flash player, and submit a new patch for review if it works. Thanks a lot for the advice.

[Hui Huang]

Symbian Flash Player has been working OK in windowed mode with S60 OSS Browser, which is a Webkit Browser. Windowless mode is better than windowed mode, but unlike Adode Flash Player on Windows and X11, Symbian Flash Player can ignore wmode parameter value and always run in windowless mode if it needs to. Setting wmode to "opaque" is not necessary for Symbian Flash Player. Introducing platform specific code is not the best practice but in this case it may be better than adding a new parameter or getting wmode from DOM.

[Hui Huang]

Created attachment 63854 [details]
Proposed patch based on Eric's comments

Submit new patch for review. Tested successfully with Symbian flash player on S60 emulator.

[Simon Hausmann]

While we could do this, it makes QGraphicsWebView on Symbian incompatible with the other platforms, where we are making an effort to support application side composition.

This change _does_ break any existing browser UI based on QGraphicsView.

Are you _SURE_ that you want that?

I am not, which is why I'm not in favour of this patch altogether. I think efforts are better spent on making Flash on Symbian support windowless rendering.

[Hui Huang]

Thanks for the comments. I agree with you that windowless mode is better. However, at this point, it is impossible to convince Symbian Flash Player to drop windowed mode completely, which is their default mode with better performance and is working OK with S60 OSS Browser. In addition, unlike Adobe Flash Player, Symbian Flash Player can ignore wmode parameter value and run in windowless mode as default, so the problem of windowed mode breaking Browser UI can be solved from Plug-in side on Symbian if needed. I don't think Symbian specific implementation is the best approach, but I can't find a better solution in this case and we need to meet the deadline for product release. Your comments and proposal are appreciated.

[Simon Hausmann]

(In reply to comment #11)
> Thanks for the comments. I agree with you that windowless mode is better. However, at this point, it is impossible to convince Symbian Flash Player to drop windowed mode completely, which is their default mode with better performance and is working OK with S60 OSS Browser. In addition, unlike Adobe Flash Player, Symbian Flash Player can ignore wmode parameter value and run in windowless mode as default, so the problem of windowed mode breaking Browser UI can be solved from Plug-in side on Symbian if needed. I don't think Symbian specific implementation is the best approach, but I can't find a better solution in this case and we need to meet the deadline for product release. Your comments and proposal are appreciated.

Before reviewing the patch, I'd like to ask: Have you tried flash with your patch applied inside the QGraphicsWebView based browser or QtTestBrowser?

[Hui Huang]

Hi Simon, it was tested with Nokia Flash player on Nokia Browser 8.x Alpha, which is a QGraphicsWebView based browser. Thanks.

[Girish Ramakrishnan]

(In reply to comment #13)
> Hi Simon, it was tested with Nokia Flash player on Nokia Browser 8.x Alpha, which is a QGraphicsWebView based browser. Thanks.

What won't work is QGV features like rotation, scaling etc. Maybe that's not important on Symbian?

[Simon Hausmann]

Comment on attachment 63854 [details]
Proposed patch based on Eric's comments

Well well, I'm pretty sure we'll end up reverting this in a little while, but for the moment this patch doesn't cause any problems on platforms other than Symbian.

[WebKit Commit Bot]

Comment on attachment 63854 [details]
Proposed patch based on Eric's comments

Clearing flags on attachment: 63854

Committed r69396: <http://trac.webkit.org/changeset/69396>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Hui Huang]

Hi Simon, thanks a lot for reviewing my patch. This is what Nokia Flash player wanted, though I personally agree with you that Windowless mode works better with QGraphicsView. As you said, we can always revert this change if Flash player changes their minds.

[Ademar Reis]

Revision r69396 cherry-picked into qtwebkit-2.1 with commit e36bbba <http://gitorious.org/webkit/qtwebkit/commit/e36bbba>