152334 - some plugins (like WMP) don't understand the DATA attribute on the OBJECT tag

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Arun Ranganathan]

Currently, if I were to invoke an AVI file by say Windows Media Player, and if I
wished to use the OBJECT tag, I could write markup such as this:

<object type="application/x-mplayer2"
src="http://www.geocities.com/MotorCity/Pit/3571/martial/mikemookjong2.avi"height="500"
width="500"></object>

The Windows Media Player would get invoked and play the AVI file.  But the
problem is that the HTML 401 spec. doesn't have any room for the 'src' attribute
of an OBJECT tag.  Instead, you're supposed to use the 'data' attribute.  So
this is a bug to ensure we use 'data' and not 'src' to be more compliant with
HTML 401.

Comment 1

[Arun Ranganathan]

Needless to say, our OBJECT tag implementation isn't complete till we do this
kind of thing.

Comment 2

[Peter Lubczynski]

Arun,

This is not our bug. I stepped through our code in the debugger and confirm we
are correctly opening the initial stream for WMP when using the DATA attribute
with an OBJECT tag, just like is done for EMBED. It's up to the plugin to decide
what it does with the attribute/PARAM array we pass it. For example, Flash works
correctly in this case:

<object
  data="http://www.macromedia.com/shockwave/download/triggerpages/flash.swf"   
     
  type=application/x-shockwave-flash 
  width=300 height=120 loop=true>
</object> 

This is a bug in WMP because it is not doing anything with the data stream we
are sending it because it doesn't see a 'SRC' attribute in the combined array of
attributes and PARAM tags we pass the plugin. I think Real and possibly other
plugins may also have this same problem.

You can't add arbitrary attributes to the OBJECT tag, but you CAN add PARAM tags
which internally the plugin see are equivelent. Here's an example of how to get
WMP working with the OBJECT tag:

<object type="application/x-mplayer2"
        height="500"
        width="500">
  <PARAM name="src"          
value="http://www.geocities.com/MotorCity/Pit/3571/martial/mikemookjong2.avi">
</object>

In this example, since 'SRC' is a PARAM value and shows up in the array we pass
the plugin, WMP see it and works as expected.

--->reassign to EVANGELISM
I think Microsoft (and other plugin vendors) should update their plugin to
correctly look for a 'DATA' or 'SRC' attribute.

Another very hacky option to fix this problem is to artificially add a 'SRC'
entry to the array we pass the plugin if only a 'DATA' was found in the OBJECT
tag. However, I'm afraid we may set a precedent with this quirk that may cause
problems down the line.

Comment 3

[Arun Ranganathan]

Wow, great analysis!  I guess I stand corrected.  Thanks.  In fact, upon closer
study of sites using ActiveX Flash (e.g. http://www.macromedia.com , which
provides both an EMBED and an OBJECT) I find that "data" isn't a widely used
attribute.  Flash for IE is actually invoked like this:

<object classid="clsid:someLongUglyUID" codebase="http://link.to.cab#5.0.3">
<param name="movie" value="../someSWF.swf">
</object> 

So here, the param with name "movie" is used to play the SWF.  Pretty
interesting -- I had no idea that this was a plugin-determined model.  So yup,
it's evangelism, because plugins themselves should be loaded not via a param but
via the data attribute.  Does anyone disagree?  Am I reading HTML 4.01 spec.
correctly? I'm referring to:
http://www.w3.org/TR/REC-html40/struct/objects.html#edef-OBJECT

Comment 4

[Peter Lubczynski]

--->back to plugins and nominating

my bad, we've gota fix this. I just check 4.x and it does this fixup I was
talking about, converting 'data' to 'src' in the array passed to the plugin. We
should be doing the same thing or similar for compatibility.

This works in 4.x:

<object type="application/x-mplayer2"
data="http://www.geocities.com/MotorCity/Pit/3571/martial/mikemookjong2.avi"
height="500" width="500"></object>

So we should make it work with Mozilla as well.

Comment 5

[av (gone)]

This is going to be easy one, like in
|nsPluginHostImpl::InstantiateEmbeddedPlugin| change this block of code

    if(pti) {
      const char *value;
      if(tagType == nsPluginTagType_Embed)
        havedata = NS_SUCCEEDED(pti->GetAttribute("SRC", &value));
      if(tagType == nsPluginTagType_Object)
        havedata = NS_SUCCEEDED(pti->GetAttribute("DATA", &value));
    }
to
    if(pti) {
      const char *value;
      havedata = NS_SUCCEEDED(pti->GetAttribute("SRC", &value));
      if (!havedata)
      havedata = NS_SUCCEEDED(pti->GetAttribute("DATA", &value));
    }

This will make SRC and DATA fully interchangeable in both OBJECT and EMBED tags
or we can limit this to the OBJECT tag only. But in any case this will probably
trigger the holy war of being_smart_and_fixing_other_people_errors against
follow_the_standards.

Comment 6

[Peter Lubczynski]

Andrei, what I observed in 4.x was that the entry for the string 'data' in the
array passed to the plugin in NPP_New was changed to say 'src'. I think we
should either do the same thing or extend the array with another entry. Thoughts?

Comment 7

[Peter Lubczynski]

Attachment: patch v.1

Here's a patch to get WMP and Real working with the OBJECT tag using W3C specs.
This patch simply copies a "data" attribute to a "src" entry in the array of
attributes/parameters we pass the plugin. Please review.

Comment 8

[av (gone)]

Comment on attachment 88490 [details] [diff] [review]
patch v.1

-    mNumCachedParams = 0xFFFF;
+    mNumCachedAttrs = 0xFFFE; 

Was it a mistake in the present code?

I have a concern about the whole approach. SRC is not listed as a standard
reserve attribute name for the object tag. This means  that content developer
may use it just like any other custom attribute. With this patch we are going
to ignore DATA attribute if it is also present. Is this my understanding
correct?

Comment 9

[Peter Lubczynski]

> Was it a mistake in the present code?

Appears to have been.

> With this patch we are going to ignore DATA attribute if it is also present.

We will not override a 'custom' SRC attribute if the author specified one. We
will only add a SRC if there is a DATA on the OBJECT tag and there is no SRC
attriubte.

Comment 10

[Peter Lubczynski]

> Was it a mistake in the present code?

Appears to have been.

> With this patch we are going to ignore DATA attribute if it is also present.

We will not override a 'custom' SRC attribute if the author specified one. We
will only ADD a SRC if there is a DATA on the OBJECT tag and there is no SRC
attriubte. This SRC will be a copy of DATA. The patch will not change what the
page author has specified in any way.

Comment 11

[av (gone)]

Comment on attachment 88490 [details] [diff] [review]
patch v.1

r=av

We should probably mention somewhere that we imply a specific meaning of the
SRC attribute, and we always have as far as I understand. With OBJECT tag which
contains SRC but does not contain DATA attributes we still imply that we have
data source which does not explicitely follows from the HTML spec or NPAPI doc.

Comment 12

[rubydoo123]

this should have been marked as nsbeta1+ already, added ADT1 - this is critical
for standards compliance

Comment 13

[Jaime Rodriguez, Jr.]

lowering impact to adt2 RRTm. standards complienace it good to have, but this
would only be an ADT1 if this affected a large number of people (top sites) and
was severe (gross loss of functionality, dataloss, etc.).

Comment 14

[rubydoo123]

moving back to ADT1, this is critical for standards compliance in that sites
that use the object element will not render plug-ins -- and that is a very
serious issue, this would force authors to include the Netscape embed extention.

Comment 15

[Jaime Rodriguez, Jr.]

back to adt2. pls do not change ADT impact markings, until you have talked to
the ADT. thanks!

Comment 16

[Chris Waterson]

Comment on attachment 88490 [details] [diff] [review]
patch v.1

rs=waterson, I have no idea what this will break.

Comment 17

[Peter Lubczynski]

patch in trunk, marking FIXED

Comment 18

[Peter Lubczynski]

this is ready to be verified on the trunk, nominating for branch

Comment 19

[shrirang khanzode]

verified on trunk 070108 on NT. Object tag's data attrib works fine from within 
the browser. tried a few testcases.
sample:

<html>
<body>

<object type="application/x-mplayer2"
data="http://www.geocities.com/MotorCity/Pit/3571/martial/mikemookjong2.avi"heig
ht="500"
width="500">
</object>

</body>
</html>

Comment 20

[scottputterman]

adding adt1.0.1+.  Please get drivers approval and try to get this in to
Wednesday's branch build.

Comment 21

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 22

[Peter Lubczynski]

fixed1.0.1

Comment 23

[shrirang khanzode]

verified on 0723 branch