Bug 105135 – [Qt/Win] Flash opened in windowed mode when 'wmode' attribute set to 'default' explicitly

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Mozhaev Grigory]

Created attachment 179662 [details]
Incorrect alignment/layered of flash plugin in QGraphics(Scene/View)

As known 'default' mode for flash is 'window', thus if wmode isn't set explicity then qtwebkit try to set it to opaque (windowless).
When html code has 'wmode' already presented and it's not a 'window', then qtwebkit just passes this and doesn't change anything (lines 1535-1536 below):

FrameLoaderClientQt.cpp:
----
1532: if (wmodeIndex == WTF::notFound) {
1533:    params.append("wmode");
1534:    values.append("opaque");
1535: } else if (equalIgnoringCase(values[wmodeIndex], "window"))
1536:    values[wmodeIndex] = "opaque";
----

The result of this is incorrect alignment into QGraphics(Scene/View) and layered over it as separate window (see qgraphicswebview_windowed_flash.png attachment).

Link to reproduce the problem:
http://www.bbc.co.uk/news/world-latin-america-18922704

(btw, any video there has the same problem)

[Mozhaev Grigory]

Created attachment 179663 [details]
Force flash to windowless mode

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files


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

[Mozhaev Grigory]

Created attachment 179665 [details]
Force flash to windowless mode

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/WebCoreSupport/FrameLoade..." exit_code: 1
Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1540:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 1 files


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

[Mozhaev Grigory]

Created attachment 179666 [details]
Force flash to windowless mode

[Mozhaev Grigory]

btw, Chromium simply force to swap wmode to opaque with no matter what was there before, see:

http://code.google.com/p/chromiumembedded/source/browse/trunk/libcef/browser_webview_delegate.cc?r=475#629

Maybe we shall not check wmode value at all and do the same?

[Allan Sandfeld Jensen]

Comment on attachment 179666 [details]
Force flash to windowless mode

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

I am not sure about the behaviour change yet, but as a minimum it needs to be identical in WebKit2 in WebFrameLoaderClient.cpp

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1537
> +            if ((equalIgnoringCase(values[wmodeIndex], "window")) || (equalIgnoringCase(values[wmodeIndex], "default")))

Wrong indentation.

[Mozhaev Grigory]

Finally we made a decision to have the following condition (near to which chrome has):

+ } else if (!equalIgnoringCase(values[wmodeIndex], "direct"))
+               values[wmodeIndex] = "opaque";

otherwise there is a cases when 'default' means by different aliases on different sites (or even absence of wmode means as 'default' on some of them) and we still get windowed flash. On the other side the 'direct' should be always windowed (at least it needs special direct painting surface which qt can't provide this time on graphicsscene, correct me if I'm wrong), but this is different issue.


(In reply to comment #7)
> (From update of attachment 179666 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179666&action=review
> 
> I am not sure about the behaviour change yet, but as a minimum it needs to be identical in WebKit2 in WebFrameLoaderClient.cpp
> 
> > Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1537
> > +            if ((equalIgnoringCase(values[wmodeIndex], "window")) || (equalIgnoringCase(values[wmodeIndex], "default")))
> 
> Wrong indentation.