Bug 50495 - [Qt] wmode parameter for flash plugins always gets overridden to opaque on QGraphicsWebView
Summary: [Qt] wmode parameter for flash plugins always gets overridden to opaque on QG...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
: 39569 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-03 15:20 PST by Viatcheslav Ostapenko
Modified: 2011-03-01 20:04 PST (History)
12 users (show)

See Also:


Attachments
Check, that wmode is really "window" and do not override transparent wmode. (1.50 KB, patch)
2010-12-03 16:09 PST, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Fix style error (1.50 KB, patch)
2010-12-03 16:20 PST, Viatcheslav Ostapenko
vestbo: review+
Details | Formatted Diff | Diff
Extra hunk removed and patch rebased to updated source tree. (1.30 KB, patch)
2011-02-10 10:58 PST, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Add "reviewed by" (1.30 KB, patch)
2011-02-10 13:49 PST, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Add bug id to changelog (1.35 KB, patch)
2011-02-10 15:22 PST, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Viatcheslav Ostapenko 2010-12-03 15:20:25 PST
"wmode" parameter for flash plugins always gets overridden to opaque on QGraphicsWebView and transparent plugins do not work.
Comment 1 Viatcheslav Ostapenko 2010-12-03 16:09:10 PST
Created attachment 75572 [details]
Check, that wmode is really "window" and do not override transparent wmode.
Comment 2 WebKit Review Bot 2010-12-03 16:11:29 PST
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.
Comment 3 Viatcheslav Ostapenko 2010-12-03 16:20:57 PST
Created attachment 75577 [details]
Fix style error
Comment 4 Kenneth Rohde Christiansen 2010-12-06 01:33:44 PST
(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.
Comment 5 Viatcheslav Ostapenko 2010-12-07 07:41:20 PST
(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.
Comment 6 Eric Seidel (no email) 2010-12-12 02:25:44 PST
I don't understand plugins.  But I know there are people who do. :)
Comment 7 Girish Ramakrishnan 2010-12-24 00:31:26 PST
(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.
Comment 8 Tor Arne Vestbø 2010-12-24 06:20:54 PST
Comment on attachment 75577 [details]
Fix style error

r- per Girish's comment
Comment 9 Viatcheslav Ostapenko 2010-12-24 11:20:53 PST
(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.
Comment 10 Girish Ramakrishnan 2010-12-27 09:44:11 PST
(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!
Comment 11 Viatcheslav Ostapenko 2010-12-27 21:55:49 PST
> 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.
Comment 12 Viatcheslav Ostapenko 2011-01-26 12:00:07 PST
> 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.
Comment 13 Viatcheslav Ostapenko 2011-02-10 08:07:15 PST
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.
Comment 14 Tor Arne Vestbø 2011-02-10 10:41:23 PST
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
Comment 15 Viatcheslav Ostapenko 2011-02-10 10:58:45 PST
Created attachment 82007 [details]
Extra hunk removed and patch rebased to updated source tree.
Comment 16 Viatcheslav Ostapenko 2011-02-10 13:49:42 PST
Created attachment 82042 [details]
Add "reviewed by"
Comment 17 WebKit Review Bot 2011-02-10 13:51:01 PST
Comment on attachment 82042 [details]
Add "reviewed by"

Rejecting attachment 82042 [details] from commit-queue.

ostapenko.viatcheslav@nokia.com 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.
Comment 18 Viatcheslav Ostapenko 2011-02-10 15:08:12 PST
Comment on attachment 82042 [details]
Add "reviewed by"

Missing bug id
Comment 19 Viatcheslav Ostapenko 2011-02-10 15:22:31 PST
Created attachment 82060 [details]
Add bug id to changelog
Comment 20 WebKit Commit Bot 2011-02-11 11:35:38 PST
The commit-queue encountered the following flaky tests while processing attachment 82060 [details]:

inspector/debugger-breakpoints-not-activated-on-reload.html bug 54300 (author: podivilov@chromium.org)
The commit-queue is continuing to process your patch.
Comment 21 WebKit Commit Bot 2011-02-11 11:37:50 PST
Comment on attachment 82060 [details]
Add bug id to changelog

Clearing flags on attachment: 82060

Committed r78352: <http://trac.webkit.org/changeset/78352>
Comment 22 WebKit Commit Bot 2011-02-11 11:37:57 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 WebKit Review Bot 2011-02-11 12:43:26 PST
http://trac.webkit.org/changeset/78352 might have broken GTK Linux 64-bit Debug
Comment 24 Nancy Piedra 2011-02-15 07:26:20 PST
Adding to critical master.  This is required for flash.
Comment 25 Ademar Reis 2011-02-17 12:44:47 PST
Revision r78352 cherry-picked into qtwebkit-2.1.x with commit 9b0c70f <http://gitorious.org/webkit/qtwebkit/commit/9b0c70f>
Comment 26 Viatcheslav Ostapenko 2011-03-01 20:04:19 PST
*** Bug 39569 has been marked as a duplicate of this bug. ***