[webkit-reviews] review granted: [Bug 60726] WebKit2: Implement plugin quirk for Mozilla user agent for older Flash : [Attachment 93337] [PATCH] Fix

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 60726: WebKit2: Implement plugin quirk for Mozilla user agent for older
Flash
https://bugs.webkit.org/show_bug.cgi?id=60726

Attachment 93337: [PATCH] Fix
https://bugs.webkit.org/attachment.cgi?id=93337&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
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.