Bug 49016 - REGRESSION (r70748): latest nightly builds kills AC_QuickTime.js
: REGRESSION (r70748): latest nightly builds kills AC_QuickTime.js
Status: RESOLVED FIXED
: WebKit
Plug-ins
: 528+ (Nightly build)
: Macintosh Intel Mac OS X 10.6
: P1 Major
Assigned To:
: https://tera.usg.utk.edu/PAMS20101104...
: InRadar, Regression
: 57071
: 57132
  Show dependency treegraph
 
Reported: 2010-11-04 12:21 PST by
Modified: 2011-03-25 17:14 PST (History)


Attachments
Webkit errror (474.76 KB, image/png)
2010-11-05 10:13 PST, Michael McGuire
no flags Details
Safari working (464.31 KB, image/png)
2010-11-05 10:13 PST, Michael McGuire
no flags Details
Webkit Version (32.74 KB, image/png)
2010-12-01 13:35 PST, Michael McGuire
no flags Details
Patch (88.67 KB, patch)
2011-03-23 19:14 PST, Andy Estes
no flags Review Patch | Details | Formatted Diff | Diff
Patch (62.23 KB, patch)
2011-03-25 15:45 PST, Andy Estes
adele: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-11-04 12:21:47 PST
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
------- Comment #1 From 2010-11-05 09:29:00 PST -------
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
------- Comment #2 From 2010-11-05 10:13:24 PST -------
Created an attachment (id=73075) [details]
Webkit errror
------- Comment #3 From 2010-11-05 10:13:52 PST -------
Created an attachment (id=73076) [details]
Safari working
------- Comment #4 From 2010-11-05 10:14:51 PST -------
I believe that it is The AC_QuickTime.js script is not loaded. This has always work until this week..
------- Comment #5 From 2010-12-01 13:35:17 PST -------
Created an attachment (id=75316) [details]
Webkit Version

Webkit Version
------- Comment #6 From 2010-12-01 14:00:01 PST -------
(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>
------- Comment #7 From 2010-12-01 15:13:06 PST -------
(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.
------- Comment #8 From 2010-12-01 15:28:44 PST -------
CC'ing Andy, who would know about object vs. embed precedence.
------- Comment #9 From 2010-12-02 07:15:38 PST -------
(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.
------- Comment #10 From 2010-12-02 09:16:56 PST -------
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.
------- Comment #11 From 2010-12-02 09:27:27 PST -------
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.
------- Comment #12 From 2010-12-02 17:44:30 PST -------
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).
------- Comment #13 From 2010-12-02 18:06:28 PST -------
I'm assigning this to myself. I broke it; I bought it.
------- Comment #14 From 2010-12-02 18:07:33 PST -------
<rdar://problem/8724339>
------- Comment #15 From 2011-03-23 19:14:08 PST -------
Created an attachment (id=86733) [details]
Patch
------- Comment #16 From 2011-03-23 19:46:26 PST -------
(From update of attachment 86733 [details])
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.
------- Comment #17 From 2011-03-24 07:11:00 PST -------
(From update of attachment 86733 [details])
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)
------- Comment #18 From 2011-03-24 10:29:10 PST -------
(From update of attachment 86733 [details])
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.
------- Comment #19 From 2011-03-24 16:23:44 PST -------
Attachment 86733 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8235650
------- Comment #20 From 2011-03-24 16:35:15 PST -------
Committed r81916: <http://trac.webkit.org/changeset/81916>
------- Comment #21 From 2011-03-24 16:43:18 PST -------
(In reply to comment #19)
> Attachment 86733 [details] [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
------- Comment #22 From 2011-03-24 17:21:26 PST -------
http://trac.webkit.org/changeset/81916 might have broken Windows 7 Release (Tests)
The following tests are not passing:
fast/images/embed-image.html
------- Comment #23 From 2011-03-24 19:56:10 PST -------
I rolled this out in <http://trac.webkit.org/changeset/81933>. I need to re-think the test.
------- Comment #24 From 2011-03-25 15:45:45 PST -------
Created an attachment (id=86988) [details]
Patch
------- Comment #25 From 2011-03-25 15:47:12 PST -------
(In reply to comment #24)
> Created an attachment (id=86988) [details] [details]
> Patch

Same patch as before, but with a test that will pass on all platforms that support TestNetscapePlugin.
------- Comment #26 From 2011-03-25 16:08:03 PST -------
Committed r82001: <http://trac.webkit.org/changeset/82001>
------- Comment #27 From 2011-03-25 17:03:45 PST -------
http://trac.webkit.org/changeset/82001 might have broken Leopard Intel Release (Tests)