Bug 60726 - REGRESSION (WebKit2): Flash 9 never uses windowless mode
Summary: REGRESSION (WebKit2): Flash 9 never uses windowless mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Brian Weinstein
URL:
Keywords:
Depends on:
Blocks: 46399
  Show dependency treegraph
 
Reported: 2011-05-12 14:06 PDT by Brian Weinstein
Modified: 2011-05-13 06:45 PDT (History)
1 user (show)

See Also:


Attachments
[PATCH] Fix (4.51 KB, patch)
2011-05-12 14:11 PDT, Brian Weinstein
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>