Bug 49016 – REGRESSION (r70748): latest nightly builds kills AC_QuickTime.js

• Component: Plug-ins
• Status: RESOLVED FIXED

[Michael McGuire]

The latest nightly build kills AC_QuickTime.js and will not allow it to run and play the movie in the QuickTime player. This works in IE on Windows & and in 5.0.2 Safari on OS X and in FireFox on OS X but not in webkit. It did until today.

http://www.apple.com/library/quicktime/scripts/ac_quicktime.js

[Jer Noble]

Would you please describe the symptoms more explicitly?  On the testcase page, I see a QuickTime movie with an HREF parameter which, when clicked, opens the HREF URL in QuickTime Player.

In the WebKit nightly build, which of the following occurs?:

*) The AC_QuickTime.js script is not loaded
*) The script loads but the object tag is not created 
*) The object tag is created, but the movie does not load
*) The movie loads, but clicking on the movie does nothing

[Michael McGuire]

Created attachment 73075 [details]
Webkit errror

[Michael McGuire]

Created attachment 73076 [details]
Safari working

[Michael McGuire]

I believe that it is The AC_QuickTime.js script is not loaded. This has always work until this week..

[Michael McGuire]

Created attachment 75316 [details]
Webkit Version

Webkit Version

[Eric Carlson]

(In reply to comment #1)
> Would you please describe the symptoms more explicitly?  On the testcase page, I see a QuickTime movie with an HREF parameter which, when clicked, opens the HREF URL in QuickTime Player.
> 
> In the WebKit nightly build, which of the following occurs?:
> 
> *) The AC_QuickTime.js script is not loaded
> *) The script loads but the object tag is not created 
> *) The object tag is created, but the movie does not load
> *) The movie loads, but clicking on the movie does nothing

The AC_QuickTime.js generated markup is supposed to open the url in QuickTime Player when the user clicks on the poster:

<object classid="clsid:02BF25D5-8C17-4B23-BC80-D3488ABDDC6B" width="200px" height="100px" codebase="http://www.apple.com/qtactivex/qtplugin.cab#version=6,0,2,0">
	<param name="src" value="https://tera.usg.utk.edu/graphics/click_me.png">
	<param name="target" value="quicktimeplayer">
	<param name="controller" value="true">
	<param name="href" value="https://tera.usg.utk.edu/movies/PAMS20101104Ref.mov">
	<embed src="https://tera.usg.utk.edu/graphics/click_me.png" width="200px" height="100px" pluginspage="http://www.apple.com/quicktime/download/" target="quicktimeplayer" controller="true" href="https://tera.usg.utk.edu/movies/PAMS20101104Ref.mov">
</object>

[Jer Noble]

(In reply to comment #4)
> I believe that it is The AC_QuickTime.js script is not loaded. This has always work until this week..

It appears AC_QuickTime.js is loaded correctly, and correctly generates a nested <object> and <embed> tag.

But it appears that Safari 5.0.2 is loading the <object> tag, while WebKit is loading the <embed> tag.  The <embed> tag, however, does not use the QuickTime plugin to render the click_me.png file.

As a workaround, instead of using a .png file for your poster frame, wrap the poster frame in a QuickTime movie through QuickTime 7 Pro.  (Just drag the .png file on the QTP7 icon, then chose FIle->Save As... in the resulting window.)  This will force the <embed> tag to use the QuickTIme plugin to render the poster frame, and the href behavior will work as expected.

[Alexey Proskuryakov]

CC'ing Andy, who would know about object vs. embed precedence.

[Eric Carlson]

(In reply to comment #7)
> (In reply to comment #4)
> > I believe that it is The AC_QuickTime.js script is not loaded. This has always work until this week..
> 
> It appears AC_QuickTime.js is loaded correctly, and correctly generates a nested <object> 
> and <embed> tag.
> 
> But it appears that Safari 5.0.2 is loading the <object> tag, while WebKit is loading the <embed> 
> tag.  The <embed> tag, however, does not use the QuickTime plugin to render the click_me.png file.
> 

This is caused by http://trac.webkit.org/changeset/66992. The <object> doesn't have a type <param> and src points to a png so WebKit uses the <embed> "fallback" and renders the png itself.

> As a workaround, instead of using a .png file for your poster frame, wrap the poster frame in 
> a QuickTime movie through QuickTime 7 Pro.  (Just drag the .png file on the QTP7 icon, then chose FIle->Save As... in the resulting window.)  This will force the <embed> tag to use the QuickTIme plugin to 
> render the poster frame, and the href behavior will work as expected.

Another workaround is to include a type <param> in the <object>.

I suspect that this used to work because of the @classid. "clsid:02BF25D5-8C17-4B23-BC80-D3488ABDDC6B" uniquely identified the QuickTime plug-in, so WebKit could restore the old behavior by not rendering fallback content when an <object> has a "well known" @classid.

[Alexey Proskuryakov]

Regardless of whether there are workarounds, I don't think we should break a script advertised at www.apple.com and undoubtedly used on many sites.

[Michael McGuire]

The fix for me was to delete the current build of WK and replace it with an older version (3/10). WK would not run BUT Safari (5.0.3) worked as it should downloading the .js file and running the movie. I have since deleted WK from my box and rebooted and Safari could not be happier.

[Andy Estes]

I believe this actually regressed after http://trac.webkit.org/changeset/70748. This is the changeset that removed all of our mappings from classid to MIME type and forced fallback content to be rendered when there is a non-empty classid attribute.

Falling back in this case is the right thing to do, but it exposes a bug in the way we handle embed elements. Other browsers see the embed element and ask their plug-in database for a plug-in that can handle either the MIME type specified by the type attribute or the file extension of the src attribute. In this case, QuickTime says it can handle .png files and is loaded (although relying on this seems brittle; what if another plug-in claims to support PNGs?).

WebKit does things differently. It shares code with the object element which allows it to load images and nested browsing contexts (iframes) as well as plug-ins. WebKit sees that the src attribute points to an image and completely bypasses the plug-in database, rendering the image natively as if it were specified by an <img>.

I think this behavior is wrong. HTML5 specifies that object elements can load images, iframes and plug-ins, but it restricts embed to plug-ins alone. If we followed that, we'd let QuickTime claim support for .png files, pass all the other embed attributes to the plug-in, and things would work as expected (unless another plug-in lays claim to .png, but that's another issue).

[Andy Estes]

I'm assigning this to myself. I broke it; I bought it.

[Andy Estes]

<rdar://problem/8724339>

[Andy Estes]

Created attachment 86733 [details]
Patch

[Darin Adler]

Comment on attachment 86733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86733&action=review

Patch looks good. I did not have time to review the whole thing, but I have one comment right now.

> Source/WebCore/html/HTMLPlugInImageElement.cpp:45
> +    // For Gecko compatibility, prefer embeds to use plugins for loading images
> +    // instead of native image rendering.
> +    , m_preferPluginsForImages(hasTagName(HTMLNames::embedTag))

It seems suboptimal for this class to set the policy based on the tag name. Instead there should be a constructor argument to control the policy.

[Eric Carlson]

Comment on attachment 86733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86733&action=review

> Source/WebCore/loader/SubframeLoader.cpp:107
> +         // Application plugins are plugins implemented by the user agent, for example Qt plugins,
> +         // as opposed to third-party code such as flash. The user agent decides whether or not they are
> +         // permitted, rather than WebKit.

I realize that this isn't new, but as long as you are modifying the function, "flash" should be capitalized and "plugin" should be hyphenated, "plug-in" (in comments anyway)

[Darin Adler]

Comment on attachment 86733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86733&action=review

> Source/WebCore/html/HTMLPlugInImageElement.h:46
> +    bool preferPluginsForImages() const { return m_preferPluginsForImages; }

For boolean getters like this we normally try to avoid things that can be interpreted as verb phrases. That’s why you see the name “should” so often.

So I suggest shouldPreferPlugInsForImages and m_shouldPreferPlugInsForImages.

> Source/WebCore/loader/FrameLoader.cpp:1000
> +    bool isSupportedPluginMIMEType = PluginDatabase::installedPlugins()->isMIMETypeRegistered(mimeType);

The word “supported” sounds like a statement about WebKit rather than the plug-ins.

[WebKit Review Bot]

Attachment 86733 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8235650

[Andy Estes]

Committed r81916: <http://trac.webkit.org/changeset/81916>

[Andy Estes]

(In reply to comment #19)
> Attachment 86733 [details] did not build on chromium:
> Build output: http://queues.webkit.org/results/8235650

This patch landed before the EWS warning. Build fixed in http://trac.webkit.org/changeset/81917

[WebKit Review Bot]

http://trac.webkit.org/changeset/81916 might have broken Windows 7 Release (Tests)
The following tests are not passing:
fast/images/embed-image.html

[Andy Estes]

I rolled this out in <http://trac.webkit.org/changeset/81933>. I need to re-think the test.

[Andy Estes]

Created attachment 86988 [details]
Patch

[Andy Estes]

(In reply to comment #24)
> Created an attachment (id=86988) [details]
> Patch

Same patch as before, but with a test that will pass on all platforms that support TestNetscapePlugin.

[Andy Estes]

Committed r82001: <http://trac.webkit.org/changeset/82001>

[WebKit Review Bot]

http://trac.webkit.org/changeset/82001 might have broken Leopard Intel Release (Tests)