Summary: | Implement plugin quirks in WebKit2 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | ahmad.saleem792, ap, bweinstein, rniwa | ||||
Priority: | P1 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Bug Depends on: | 60726, 60792, 60795, 60803 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2010-09-23 13:22:00 PDT
Created attachment 93333 [details]
[PATCH] Mozilla User Agent Quirk
Comment on attachment 93333 [details] [PATCH] Mozilla User Agent Quirk View in context: https://bugs.webkit.org/attachment.cgi?id=93333&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 on attachment 93333 [details] [PATCH] Mozilla User Agent Quirk Not putting patches in this bug - using child bugs and treating this as an umbrella bug. See https://bugs.webkit.org/show_bug.cgi?id=60726 for this patch. Here are all the plugin quirks that WebKit1 uses on Windows, and the revisions in which they were added. http://trac.webkit.org/browser/trunk/Source/WebCore/plugins/win/PluginPackageWin.cpp?rev=86242#L91 PluginQuirkFlashURLNotifyBug http://trac.webkit.org/changeset/29778 PluginQuirkDeferFirstSetWindowCall http://trac.webkit.org/changeset/23424 PluginQuirkDontAllowMultipleInstances http://trac.webkit.org/changeset/30698 PluginQuirkDontCallWndProcForSameMessageRecursively http://trac.webkit.org/changeset/25642 PluginQuirkDontClipToZeroRectWhenScrolling http://trac.webkit.org/changeset/29974 PluginQuirkDontSetNullWindowHandleOnDestroy http://trac.webkit.org/changeset/30218 PluginQuirkDontUnloadPlugin http://trac.webkit.org/changeset/25606 PluginQuirkHasModalMessageLoop http://trac.webkit.org/changeset/28700 PluginQuirkRemoveWindowlessVideoParam http://trac.webkit.org/changeset/23947 PluginQuirkThrottleInvalidate http://trac.webkit.org/changeset/23441 PluginQuirkThrottleWMUserPlusOneMessages http://trac.webkit.org/changeset/25310 PluginQuirkWantsMozillaUserAgent http://trac.webkit.org/changeset/22693 This quirk doesn't exist anymore, but once did, so might warrant investigation: PluginQuirksWantsAsciiWindowProc http://trac.webkit.org/changeset/23450 We should go through the above list and test which of these quirks are needed in WebKit2. …and split each needed quirk into its own bug that blocks this one. (In reply to comment #4) > PluginQuirkWantsMozillaUserAgent > http://trac.webkit.org/changeset/22693 This is bug 60726. (In reply to comment #4) > PluginQuirkDeferFirstSetWindowCall > http://trac.webkit.org/changeset/23424 See also http://trac.webkit.org/changeset/23956 (In reply to comment #4) > PluginQuirkDontUnloadPlugin > http://trac.webkit.org/changeset/25606 See also http://trac.webkit.org/changeset/26898 (In reply to comment #9) > (In reply to comment #4) > > PluginQuirkDontUnloadPlugin > > http://trac.webkit.org/changeset/25606 > > See also http://trac.webkit.org/changeset/26898 And http://trac.webkit.org/changeset/29871 (In reply to comment #4) > PluginQuirkDontClipToZeroRectWhenScrolling > http://trac.webkit.org/changeset/29974 I filed bug 60782 about the QuickTime bug that r29974 was fixing. I haven't tested yet what would happen to Java if we were to fix that bug in the same way. (In reply to comment #8) > (In reply to comment #4) > > PluginQuirkDeferFirstSetWindowCall > > http://trac.webkit.org/changeset/23424 > > See also http://trac.webkit.org/changeset/23956 WebKit2 always does the "defer first SetWindow call" behavior currently (as did WebKit1 before r23424). We haven't seen any issues with this behavior so far, so at the moment there's no reason to implement this quirk. (Maybe more testing with Flash 9 would expose an issue?) (See also bug 46716.) (In reply to comment #4) > PluginQuirkThrottleWMUserPlusOneMessages > http://trac.webkit.org/changeset/25310 This is now bug 60792. (In reply to comment #4) > PluginQuirkThrottleInvalidate > http://trac.webkit.org/changeset/23441 The original bug says that you couldn't select text at kids.yahoo.com. It was filed when Flash 9 was the newest version of Flash available. I can't repro the bug anymore (but the site has surely changed in the last 4 years). Seems pretty similar to bug 60792, though. (In reply to comment #4) > PluginQuirkFlashURLNotifyBug > http://trac.webkit.org/changeset/29778 Original repro steps: 1. Go to http://mail.yahoo.com/ and sign in 2. Click "New" button on the left 3. Type anything in the "To:" field 4. Click the Back button 5. Confirm that you want to leave the page I can't reproduce any problem using these steps with Flash 9 installed (though of course the site has changed in the last 3.5 years). Seems like we're OK without this one. (In reply to comment #11) > (In reply to comment #4) > > PluginQuirkDontClipToZeroRectWhenScrolling > > http://trac.webkit.org/changeset/29974 > > I filed bug 60782 about the QuickTime bug that r29974 was fixing. I haven't tested yet what would happen to Java if we were to fix that bug in the same way. To be clear: We definitely don't need this quirk until we fix bug 60782. If we fix bug 60782 the same way we fixed it in WebKit1 (by clipping to a zero rect while scrolling), then we'll need to test to see whether we need this quirk. (In reply to comment #4) > PluginQuirkDontAllowMultipleInstances > http://trac.webkit.org/changeset/30698 This is now bug 60795. (In reply to comment #4) > PluginQuirkDontSetNullWindowHandleOnDestroy > http://trac.webkit.org/changeset/30218 Original steps to reproduce: 1. Go to http://wdshe.jp/ghibli/special/iblard/ 2. Select Trailers 3. Select any of the WMP versions 4. Wait for the video to finish 5. Press Close This would cause a hang. That URL no longer exists. I can't reproduce a hang on http://revolunet.github.com/VLCcontrols/src/simple.html with VLC 0.8.6d or 1.1.9. Seems like we don't need this quirk for now. (In reply to comment #4) > PluginQuirkRemoveWindowlessVideoParam > http://trac.webkit.org/changeset/23947 Original steps to reproduce: 1. Go to http://www.cnbc.com/ 2. Scroll down the page 3. Click on any movie from the right side of the page This would crash. CNBC uses Flash instead of Windows Media Player now. I tried to reproduce by modifying <http://www.vdat.com/techsupport/PRNwindowstest.asp> to pass "1" for "WindowlessVideo", but couldn't reproduce a crash. However, the plugin definitely doesn't paint, so I filed bug 60803. (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #4) > > > PluginQuirkDontUnloadPlugin > > > http://trac.webkit.org/changeset/25606 Original repro steps: 1. Install Silverlight 1.0 2. Go to http://microsoft.com/silverlight/getassistance.aspx You'd crash. I can't find Silverlight 1.0 anywhere on microsoft.com anymore. That URL also no longer exists. Other Silverlight pages seem to work fine. And Anders tells me that the NPRuntime bug that is referenced in the comment for this quirk doesn't exist in WK2. So I think Silverlight is OK in WebKit2. > > See also http://trac.webkit.org/changeset/26898 Original repro steps: 1. Install Java 6 Update 3 2. Go to http://java.sun.com/products/plugin/1.4.1/demos/plugin/applets.html You'd crash. That URL no longer exists. I haven't yet tested Java 6 Update 3 on other pages. Anders says that the comment from that revision ("We purposefully do not destroy our Java VM when its reference count reaches 0") is still true. So it's possible this is still a problem for Java. > And http://trac.webkit.org/changeset/29871 Original repro steps: 1. Install RealPlayer 6.0.14 2. Go to http://www.pbs.org/wgbh/nova/genome/program_t.html 3. Click on any of the RealVideo links 4. Close the popup window You'd crash. I can't find RealPlayer 6.0.14 on any legitimate websites. The crash does not occur with RealPlayer 12.0.1.647. I guess we're probably OK. So, we need to test Java 6 Update 3 to see if it has issues. (In reply to comment #4) > PluginQuirkHasModalMessageLoop > http://trac.webkit.org/changeset/28700 Original repro steps: 1. Go to <http://www.wptv.wp.pl/news.html?wid=9256941&type=1&rfbawp=1191311831.081&ticaid=14911> 2. Click on round button below the video You'd crash. wptv now uses Flash for their videos, so this can no longer be tested. (In reply to comment #4) > PluginQuirkDontCallWndProcForSameMessageRecursively > http://trac.webkit.org/changeset/25642 Original repro steps: 1. Delete all cookies/preferences/cache 2. Go to bbcnews.co.uk 3. Scroll down to video and audio news 4. Click Watch button 5. Select RealPlayer and High quality in popup window You'd crash. BBC now uses Flash, so this can't be tested. (In reply to comment #20) > So, we need to test Java 6 Update 3 to see if it has issues. Download link: http://java.sun.com/products/archive/j2se/6u3/index.html NPAPI plugin and Webkit plugins ae not supported anymore. Can this be marked as "RESOLVED WONTFIX"? Thanks! I think Plugin support and related quirks are not needed anymore. Can we also mark other "Dependent" bug as "RESOLVED WONTFIX", since they are plugin related? Thanks! |