Bug 46399

Summary: Implement plugin quirks in WebKit2
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Plug-insAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: bweinstein
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Bug Depends on: 60792, 60795, 60803, 60726    
Bug Blocks:    
Attachments:
Description Flags
[PATCH] Mozilla User Agent Quirk none

Description Adam Roben (:aroben) 2010-09-23 13:22:00 PDT
2010-09-23 16:18:39 Adam Roben:
WebCore's plugin implementation has a set of quirks that are implemented for specific plugins. We should implement these quirks in WebKit2, too.

<rdar://problem/8470824>
Comment 1 Brian Weinstein 2011-05-12 14:02:45 PDT
Created attachment 93333 [details]
[PATCH] Mozilla User Agent Quirk
Comment 2 Adam Roben (:aroben) 2011-05-12 14:06:18 PDT
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 3 Brian Weinstein 2011-05-12 14:14:45 PDT
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.
Comment 4 Adam Roben (:aroben) 2011-05-13 06:39:15 PDT
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
Comment 5 Adam Roben (:aroben) 2011-05-13 06:43:47 PDT
We should go through the above list and test which of these quirks are needed in WebKit2.
Comment 6 Adam Roben (:aroben) 2011-05-13 06:44:06 PDT
…and split each needed quirk into its own bug that blocks this one.
Comment 7 Adam Roben (:aroben) 2011-05-13 06:44:27 PDT
(In reply to comment #4)
> PluginQuirkWantsMozillaUserAgent
> http://trac.webkit.org/changeset/22693

This is bug 60726.
Comment 8 Adam Roben (:aroben) 2011-05-13 11:22:32 PDT
(In reply to comment #4)
> PluginQuirkDeferFirstSetWindowCall
> http://trac.webkit.org/changeset/23424

See also http://trac.webkit.org/changeset/23956
Comment 9 Adam Roben (:aroben) 2011-05-13 11:23:18 PDT
(In reply to comment #4)
> PluginQuirkDontUnloadPlugin
> http://trac.webkit.org/changeset/25606

See also http://trac.webkit.org/changeset/26898
Comment 10 Adam Roben (:aroben) 2011-05-13 11:25:48 PDT
(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
Comment 11 Adam Roben (:aroben) 2011-05-13 11:50:18 PDT
(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.
Comment 12 Adam Roben (:aroben) 2011-05-13 12:59:49 PDT
(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.)
Comment 13 Adam Roben (:aroben) 2011-05-13 13:06:22 PDT
(In reply to comment #4)
> PluginQuirkThrottleWMUserPlusOneMessages
> http://trac.webkit.org/changeset/25310

This is now bug 60792.
Comment 14 Adam Roben (:aroben) 2011-05-13 13:12:44 PDT
(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.
Comment 15 Adam Roben (:aroben) 2011-05-13 13:15:42 PDT
(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.
Comment 16 Adam Roben (:aroben) 2011-05-13 13:17:42 PDT
(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.
Comment 17 Adam Roben (:aroben) 2011-05-13 13:29:31 PDT
(In reply to comment #4)
> PluginQuirkDontAllowMultipleInstances
> http://trac.webkit.org/changeset/30698

This is now bug 60795.
Comment 18 Adam Roben (:aroben) 2011-05-13 13:58:09 PDT
(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.
Comment 19 Adam Roben (:aroben) 2011-05-13 14:13:47 PDT
(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.
Comment 20 Adam Roben (:aroben) 2011-05-13 14:38:36 PDT
(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.
Comment 21 Adam Roben (:aroben) 2011-05-13 14:43:27 PDT
(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.
Comment 22 Adam Roben (:aroben) 2011-05-13 14:46:23 PDT
(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.
Comment 23 Adam Roben (:aroben) 2011-05-13 14:50:18 PDT
(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