Bug 50495 – [Qt] wmode parameter for flash plugins always gets overridden to opaque on QGraphicsWebView

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Viatcheslav Ostapenko]

"wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.

[Viatcheslav Ostapenko]

Created attachment 75572 [details]
Check, that wmode is really "window" and do not override transparent wmode.

[WebKit Review Bot]

Attachment 75572 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/qt/ChangeLog', u'WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp']" exit_code: 1
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1558:  Missing space before ( in if(  [whitespace/parens] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Viatcheslav Ostapenko]

Created attachment 75577 [details]
Fix style error

[Kenneth Rohde Christiansen]

(In reply to comment #0)
> "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.

I believe we did this on purpose to always get windowless plugins on linux.

[Viatcheslav Ostapenko]

(In reply to comment #4)
> (In reply to comment #0)
> > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> 
> I believe we did this on purpose to always get windowless plugins on linux.

But transparent plugin is also windowless. Of course it can cause some performance overhead for blending, but there are a lot of places with blending in webkit, so I don't think it's real problem.
As I understand, there are 2 options when plugin can get into windowed mode:
1. when wmode is not specified, than plugin use it's own default (windowed for current implementations of flash plugins).
2. wmode=window when page wants plugin to play in the topmost layer above other elements.

My patch fixes only transparent wmode for flash plugin, but leaves it windowless even if page specifies wmode=window.

[Eric Seidel (no email)]

I don't understand plugins.  But I know there are people who do. :)

[Girish Ramakrishnan]

(In reply to comment #0)
> "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.

The change is intentional. We want to force flash to opaque mode in QGWV in all cases.

Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?

Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.

[Tor Arne Vestbø]

Comment on attachment 75577 [details]
Fix style error

r- per Girish's comment

[Viatcheslav Ostapenko]

(In reply to comment #7)
> (In reply to comment #0)
> > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> The change is intentional. We want to force flash to opaque mode in QGWV in all cases.
> Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?
> Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.

The flash code I have just blits from blits from bitmap, so if it has alpha on this bitmap backing store, than transparency would work. At least flash guys had working test cases with transparent flash. Unfortunatelly, I'm not in office now. I'll recheck after holidays.

[Girish Ramakrishnan]

(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #0)
> > > "wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
> > The change is intentional. We want to force flash to opaque mode in QGWV in all cases.
> > Note that we cannot make flash transparency work in QGraphicsWebView because Flash has a bug wherein it does not render to 32-bit pixmaps. Do you have a new version of flash that has fixed this problem or have the latest versions of flash fixed it? Do you see transparency working after your patch?
> > Flash transparency works only with QWebView wherein I added very hairy code to grab contents from the backing store to fake transparency.
> 
> The flash code I have just blits from blits from bitmap, so if it has alpha on this bitmap backing store, than transparency would work. 

I agree that it will work if we had alpha on the pixmap that Flash would paint to. The problem is that so far all versions of Flash I have seen have not been able to render to transparent/32-bit pixmaps. Also, please do check if the Flash guys actually tested with Qt/WebKit and not a mozilla based browser. Thanks!

[Viatcheslav Ostapenko]

> I agree that it will work if we had alpha on the pixmap that Flash would paint 
> to. The problem is that so far all versions of Flash I have seen have not been > able to render to transparent/32-bit pixmaps. 

So, as I understand, they will produce some non-transparent backing store, which will be blit to qt painter normally and there is no need to override transparent wmode to opaque? This way if transparent rendering in flash plugin will be fixed, than there will be no need to change code again. 

> Also, please do check if the Flash guys actually tested with Qt/WebKit and not 
> a mozilla based browser. 

Actually, I've made this bug by their request (from flash plugin for s60 developers). They have some test cases with transparency and they fail. I'd like to figure it out myself when get back to office.

[Viatcheslav Ostapenko]

> I agree that it will work if we had alpha on the pixmap that Flash would paint 
> to. The problem is that so far all versions of Flash I have seen have not been 
> able to render to transparent/32-bit pixmaps. Also, please do check if the 
> Flash guys actually tested with Qt/WebKit and not a mozilla based browser. 
> Thanks!

Yes, new flash plugin works in transparent mode.
This test page works: http://125.16.213.243/upload/NGFlash10/AS3_FL9/Windowless/as3_opq/01_dynamic_ball.html

Also, if some flash plugin cannot handle transparent mode it can fallback to opaque mode internally. There is no need to restrict it in webkit.

[Viatcheslav Ostapenko]

Comment on attachment 75577 [details]
Fix style error

Current windowless flash plugin implementation supports transparent mode.
If some plugin doesn't support this, it could force to opaque internally.

[Tor Arne Vestbø]

Comment on attachment 75577 [details]
Fix style error

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

r=me, but remove the extra hunk

> WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1532
> +

Don't add extra hunks

[Viatcheslav Ostapenko]

Created attachment 82007 [details]
Extra hunk removed and patch rebased to updated source tree.

[Viatcheslav Ostapenko]

Created attachment 82042 [details]
Add "reviewed by"

[WebKit Review Bot]

Comment on attachment 82042 [details]
Add "reviewed by"

Rejecting attachment 82042 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Viatcheslav Ostapenko]

Comment on attachment 82042 [details]
Add "reviewed by"

Missing bug id

[Viatcheslav Ostapenko]

Created attachment 82060 [details]
Add bug id to changelog

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 82060 [details]:

inspector/debugger-breakpoints-not-activated-on-reload.html bug 54300 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 82060 [details]
Add bug id to changelog

Clearing flags on attachment: 82060

Committed r78352: <http://trac.webkit.org/changeset/78352>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/78352 might have broken GTK Linux 64-bit Debug

[Nancy Piedra]

Adding to critical master.  This is required for flash.

[Ademar Reis]

Revision r78352 cherry-picked into qtwebkit-2.1.x with commit 9b0c70f <http://gitorious.org/webkit/qtwebkit/commit/9b0c70f>

[Viatcheslav Ostapenko]

*** Bug 39569 has been marked as a duplicate of this bug. ***