Bug 60726 – REGRESSION (WebKit2): Flash 9 never uses windowless mode

• Component: WebKit2
• Status: RESOLVED FIXED

[Brian Weinstein]

Versions of flash before 10 only request windowless plugins if we return a Mozilla user agent. We should add that quirk.

[Brian Weinstein]

To test: Install Flash version < 10 (http://kb2.adobe.com/cps/142/tn_14266.html)
and test at http://communitymx.com/content/source/E5141/wmodetrans.htm. You should see a star behind the bouncing ball.

[Brian Weinstein]

Created attachment 93337 [details]
[PATCH] Fix

[Adam Roben (:aroben)]

Comment on attachment 93337 [details]
[PATCH] Fix

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

> Source/WebKit2/Shared/Plugins/Netscape/win/NetscapePluginModuleWin.cpp:126
> +    // Pre Flash v10 only requests windowless plugins if we use a Mozilla user agent.
> +
> +    // To test: Install Flash version < 10 (http://kb2.adobe.com/cps/142/tn_14266.html)
> +    // and test at http://communitymx.com/content/source/E5141/wmodetrans.htm. You should
> +    // see a star behind the bouncing ball.

I'd move these comments inside the if (mimeTypes[i].type == ...) test. That way they are closer to the code that is actually dealing with Flash.

I think it would be a little better to put the test case info in the bug and have the comment just reference the bug.

> Source/WebKit2/Shared/Plugins/Netscape/win/NetscapePluginModuleWin.cpp:129
> +        if (mimeTypes[i].type == "application/x-shockwave-flash") {

You could add a FIXME here saying that it's a little strange to assume that any plugin that handles this MIME type needs this quirk. (Maybe we should be checking the plugin's name instead, e.g.)

> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:137
> +#if PLUGIN_ARCHITECTURE(MAC)
> +        "Macintosh; U; Intel Mac OS X;"

I don't think Apple's Mac port ever had this behavior before. (Only Qt/mac and GTK+/mac did.) We should test to make sure it's really desired.

[Adam Roben (:aroben)]

Comment on attachment 93337 [details]
[PATCH] Fix

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

>> Source/WebKit2/Shared/Plugins/Netscape/win/NetscapePluginModuleWin.cpp:126
>> +        if (mimeTypes[i].type == "application/x-shockwave-flash") {
> 
> I'd move these comments inside the if (mimeTypes[i].type == ...) test. That way they are closer to the code that is actually dealing with Flash.
> 
> I think it would be a little better to put the test case info in the bug and have the comment just reference the bug.

Sorry, this comment was from the older patch attached to bug 46399.

>> Source/WebKit2/Shared/Plugins/Netscape/win/NetscapePluginModuleWin.cpp:129
>> +
> 
> You could add a FIXME here saying that it's a little strange to assume that any plugin that handles this MIME type needs this quirk. (Maybe we should be checking the plugin's name instead, e.g.)

This one too.

[Adam Roben (:aroben)]

Committed r86380: <http://trac.webkit.org/changeset/86380>