Closed Bug 277434 Opened 19 years ago Closed 18 years ago

Setting .type property on <object> element doesn't create an HTML attribute type


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: martin.honnen, Assigned: jst)



(Keywords: fixed1.8, regression)


(5 files)

When trying to set
  var object = document.createElement('object');
  object.type = 'image/x-special';
then the created object element doesn't have a type attribute, that is
This happens with 1.8a trunk builds (tested with Mozilla 1.8a6 (Mozilla/5.0
(Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20050106) as well as with
Firefox 1.0 (tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.7.5) Gecko/20041107 Firefox/1.0) but not with the Mozilla 1.7.5 release.

As for the W3C DOM specification about that property type of HTMLObjectElement at
it doesn't in any way suggest that setting the property should not change the
HTML attribute thus it seems a bug that the attribute is not set when the
property is set.

I will upload a test case that contains a static <object> and then creates two
<object> elements with script, trying to set .type on the first while calling
.setAttribute('type', ...) on the second. The script then examines all <object>
elements in the page and outputs the result of ['type'] and
.getAttribute('type'). The result with Mozilla which I think is a bug is

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
image/x-special; ["type"]: image/x-special

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
null; ["type"]: image/x-special

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
image/x-special; ["type"]: image/x-special

the correct result should be 

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
image/x-special; ["type"]: image/x-special

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
image/x-special; ["type"]: image/x-special

Checking [object HTMLObjectElement]; .tagName: OBJECT; .getAttribute("type"):
image/x-special; ["type"]: image/x-special

meaning it shouldn't matter whether the object and its type are set statically
in HTML or with setting .type or with setting .setAttribute('type', ...), all
cases getAttribute('type') and the property access .type should give the same
This is an aviary branch regression (plugin finder).  See revision 1.73 of

Assignee: general → jst
Flags: blocking1.8b?
Keywords: aviary-landing
Summary: Setting .type property on dynamically create <object> element doesn't create an HTML attribute type → Setting .type property on <object> element doesn't create an HTML attribute type
Note that <embed> has the same problem, by the way....

jst, is there a reason we're hijacking DOM methods for internal use and changing
their behavior in the process?
OS: Windows XP → All
Hardware: PC → All
Depends on: 244125
Why was that patch even needed? mType seems compleatly unused. In both classes.
And the patch is by none other then jst! What were you smoking? ;-)
mType is used by SetType().

The problem is that our the frame calls SetType() on the content node and then
wants to use GetType() to get that type back.  But it doesn't want to change the
DOM (obviosuly).  So SetType got hacked to not change the attr value....
If it desperatly needs to do this then it should use some internal interface
rather then abusing the nsIDOM ones. Maybe time to revive my
nsHTMLSharedObjectElement class...
Too late for 1.8b1, plussing for 1.8b2.
Flags: blocking1.8b?
Flags: blocking1.8b2+
Flags: blocking1.8b-
JST, are you gonna be able to get to this pretty quickly or should we try to get
it done in 1.8b3?
moving out to blocking 1.8b3
Flags: blocking1.8b3+
Flags: blocking1.8b2+
Flags: blocking1.8b-
Maybe it's just me, but it seems like this is more what we'd want than having
setting object.type set the type attribute. But I bet IE works exactly opposite
to that, so maybe we just need to revert to what we had...

The reason for the change is that we need to expose the type of the data in the
case where it's either not specified in the HTML or where what we get from the
server doesn't match what's in the HTML, and in that case it surely does not
make sense to go change the HTML type attribute... Maybe now is the time for a
'realType' property that doesn't map directly to the attribute value?
Having a new property that has the server-sent type sounds great to me.
Do we need to expose this in the DOM at all, if so .realType sounds good?
Otherwise we could just use nsIContent::SetProperty
The PFS widget needs to know the real type, so we need a new exposed property.
If biesi's creating an nsIImageLoadingContent-like interface for objects, we
should just expose it there.  Note that this interface need not have classinfo
or anything if all we're using it for is PFS.
that will of course only land for 1.9, so if people want a solution for 1.8
something else is needed in the meantime...
No reason the interface, or at least the realType part of it, couldn't land for 1.8.
How about we make .type expose what it exposes today, but setting it also always
set the HTML attribute? For that we'd still need a new interface to tell the
element about the actual type, but it wouldn't need to be scriptable since the
caller that tells the element about the actual type is nsObjectFrame.cpp. That'd
be the lowest impact fix for this I believe...
This implements what I talked about in my previous comment.
Attachment #187457 - Flags: superreview?(peterv)
Attachment #187457 - Flags: review?(bugmail)
Whiteboard: needs review
Flags: blocking-aviary1.1+
Whiteboard: needs review → [cb] ready for review? r=bugmail? sr=peterv?
Isn't this fixing just half of the problem? Getting .type will never get the
attribute value and just be an empty string in most cases.

I'm not entierly sure what we're trying to do here. If the plugin code just
needs to store a type in the contentnode then IMHO nsIContent::SetProperty is a
good solution.

Or do we want to make this behave like for example myImg.width where the
property always returns the 'real' value which only sometimes is the same as the
attribute? If so, should setting .type really null out mType? And we should
probably then return the attribute value of mType isn't set in GetType.
Comment on attachment 187457 [details] [diff] [review]
Make setting .type change the type attr

why not use nsACString& here, given that both caller and callee would prefer
Comment on attachment 187457 [details] [diff] [review]
Make setting .type change the type attr


it's been suggested to use an nsACString for mType and SetActualType which
sounds fine to me.
Attachment #187457 - Flags: review?(bugmail) → review+
Comment on attachment 187457 [details] [diff] [review]
Make setting .type change the type attr

Yeah, switch to nsACString, also maybe rename mType to mActualType.
Attachment #187457 - Flags: superreview?(peterv) → superreview+
Attachment #187457 - Flags: approval1.8b3?
Attachment #187457 - Flags: approval1.8b3? → approval1.8b3+
Fixed checked in.
Closed: 18 years ago
Resolution: --- → FIXED
Doesn't this still violate the DOM HTML spec?  Per that, the .type property
should reflect the type attribute on getting no matter what.
Reopening, since we're still not following DOM spec here.
Resolution: FIXED → ---
Unless we hear a compelling argument for blocking, let's push this out to 1.8b4.
Flags: blocking1.8b4+
Flags: blocking1.8b3-
Flags: blocking1.8b3+
Whiteboard: [cb] ready for review? r=bugmail? sr=peterv?
would consider a non-risky patch if we can get one in time for b4 but not going
to block on this.
Flags: blocking1.8b4+ → blocking1.8b4-
Keywords: regression
This restores .type to map directly to the attribute and introduces a new
.actualType property that our chrome (or content too for that matter) can QI to
to get to the actual type. This way we won't pollute the plugin element
namespace except in the case when we're dealing with missing plugins.
Attachment #193116 - Flags: superreview?(bzbarsky)
Attachment #193116 - Flags: review?(bzbarsky)
Comment on attachment 193116 [details] [diff] [review]
Expose .actualType through nsIPluginElement

>Index: content/html/content/src/nsHTMLObjectElement.cpp

> nsHTMLObjectElement::SetActualType(const nsACString& aActualType)
> {
>   mActualType = aActualType;

This lets pages set the actual type on a broken plugin, right?	Is that
desirable?  Do we need a security check here?  Or perhaps we should make
setActualType noscript and leave actualType readonly scriptable?

> //NS_IMPL_STRING_ATTR(nsHTMLObjectElement, Type, type)

> nsHTMLObjectElement::GetType(nsAString& aType)

Why not just remove the manual SetType/GetType impls and uncomment the

>Index: content/html/content/src/nsHTMLSharedElement.cpp

> nsHTMLSharedElement::GetType(nsAString& aType)
> {
>-  if (!mNodeInfo->Equals(nsHTMLAtoms::embed) || mActualType.IsEmpty()) {
>+  if (!mNodeInfo->Equals(nsHTMLAtoms::embed)) {
>     GetAttr(kNameSpaceID_None, nsHTMLAtoms::type, aType);
>   } else {
>-    CopyUTF8toUTF16(mActualType, aType);
>+    aType.Truncate();

This looks like it will always return the empty string for the one case when
type matters -- <embed>.  Again, why not remove the GetType/SetType here and
just uncomment the NS_IMPL_STRING_ATTR?

r+sr=bzbarsky with those fixed.
Attachment #193116 - Flags: superreview?(bzbarsky)
Attachment #193116 - Flags: superreview+
Attachment #193116 - Flags: review?(bzbarsky)
Attachment #193116 - Flags: review+
Changed all that and checked this patch in.
Attachment #193510 - Flags: superreview+
Attachment #193510 - Flags: review+
Attachment #193510 - Flags: approval1.8b4?
Attachment #193510 - Flags: approval1.8b4? → approval1.8b4+
Flags: blocking-aviary1.5+
is this fixed for the branch? If so, can you please add the fixed1.8 keyword?
Yeah, this is all fixed
Closed: 18 years ago18 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Removing aviary-landing keyword.
Keywords: aviary-landing
It looks like the fix for this was actually wrong.  :(  In particular, it didn't switch the other consumer that was calling SetType() to using actualType.  See bug 430424.
Component: DOM: HTML → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.