Bug 60726

Summary: REGRESSION (WebKit2): Flash 9 never uses windowless mode
Product: WebKit Reporter: Brian Weinstein <bweinstein>
Component: WebKit2Assignee: Brian Weinstein <bweinstein>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 46399    
Attachments:
Description Flags
[PATCH] Fix aroben: review+

Description Brian Weinstein 2011-05-12 14:06:11 PDT
Versions of flash before 10 only request windowless plugins if we return a Mozilla user agent. We should add that quirk.
Comment 1 Brian Weinstein 2011-05-12 14:09:07 PDT
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.
Comment 2 Brian Weinstein 2011-05-12 14:11:33 PDT
Created attachment 93337 [details]
[PATCH] Fix
Comment 3 Adam Roben (:aroben) 2011-05-12 14:14:29 PDT
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.
Comment 4 Adam Roben (:aroben) 2011-05-12 14:15:29 PDT
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.
Comment 5 Adam Roben (:aroben) 2011-05-13 06:45:03 PDT
Committed r86380: <http://trac.webkit.org/changeset/86380>