Bug 50495

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

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.

Created attachment 75577 [details] Fix style error

(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.

(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.

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

(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 on attachment 75577 [details] Fix style error r- per Girish's comment

(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.

(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!

> 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.

> 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 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 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

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

Created attachment 82042 [details] Add "reviewed by"

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 on attachment 82042 [details] Add "reviewed by" Missing bug id

Created attachment 82060 [details] Add bug id to changelog

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 on attachment 82060 [details] Add bug id to changelog Clearing flags on attachment: 82060 Committed r78352: <http://trac.webkit.org/changeset/78352>

All reviewed patches have been landed. Closing bug.

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

Adding to critical master. This is required for flash.

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

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