Last Comment Bug 92071 - HTMLSpanElement does not inherit from Core Objects
: HTMLSpanElement does not inherit from Core Objects
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla0.9.6
Assigned To: Johnny Stenback (:jst,
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2001-07-24 03:28 PDT by tfriesen
Modified: 2008-07-31 02:29 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase -- click on the text to see what's up (228 bytes, text/html)
2001-07-24 07:47 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details
Proposed fix (1.99 KB, patch)
2001-09-17 15:34 PDT, Johnny Stenback (:jst,
no flags Details | Diff | Splinter Review
Better fix. (4.97 KB, patch)
2001-11-06 03:41 PST, Johnny Stenback (:jst,
fabian: review+
jband_mozilla: superreview+
Details | Diff | Splinter Review
Same as above, but even better. (11.55 KB, patch)
2001-11-06 23:53 PST, Johnny Stenback (:jst,
no flags Details | Diff | Splinter Review

Description tfriesen 2001-07-24 03:28:25 PDT
This happens in recent builds (July 23)


<div onmouseup="alert(this.test)">DIV Tag</div>
<span onmouseup="alert(this.test)">SPAN Tag</span>



Comment 1 basic 2001-07-24 05:35:03 PDT
reporter please attach a testcase
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2001-07-24 07:47:22 PDT
Created attachment 43368 [details]
testcase -- click on the text to see what's up
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2001-07-24 07:50:48 PDT
seeing this on linux build 2001-07-23-08 as well.

Odd, since nsHTMLDivElement and nsHTMLSpanElement inherit from the same things...
Comment 4 Johnny Stenback (:jst, 2001-07-24 21:56:06 PDT
Odd, but accepting...
Comment 5 Johnny Stenback (:jst, 2001-09-03 21:06:47 PDT
Moving to mozilla0.9.5
Comment 6 Fabian Guisset 2001-09-17 04:30:01 PDT
/me would like to see the fix if you still have it, it'd be interesting :-)
Comment 7 Johnny Stenback (:jst, 2001-09-17 15:34:39 PDT
Created attachment 49666 [details] [diff] [review]
Proposed fix
Comment 8 Johnny Stenback (:jst, 2001-09-17 15:40:45 PDT
The problem here is that the span class does not have a leaf interface as its
primary interface, because of this the code that sets up the prototype chain on
DOM objects ends up not setting the prototype of the span class' prototype so
that the chain is linked togethere. Here's a image that shows the difference
between div and span, note the lack of a nsIDOMHTMLSpanElement leaf interface:

 HTMLDivElement                 HTMLSpanElement
        |                              |
nsIDOMHTMLDivElement           nsIDOMHTMLElement
        |                              |
 nsIDOMHTMLElement               nsIDOMElement
        |                              |
  nsIDOMElement                   nsIDOMNode
        |                              |
   nsIDOMNode                     nsISupports
Comment 9 Sivakiran Tummala 2001-09-17 16:07:18 PDT
is this fix going to effect any other elements from the inheritance point of view.
Comment 10 Johnny Stenback (:jst, 2001-09-17 23:33:01 PDT
Sivakiran, yes, this will affect other elements too, the elements should be:


But mozilla doesn't really support all of those correctly, so I'm not sure how
much that list means here.

Comment 11 Fabian Guisset 2001-09-18 04:59:54 PDT
i knew it! it's class info's fault! it's always class info's fault! ;-)
Comment 12 Sivakiran Tummala 2001-09-18 13:36:12 PDT
thanks johnny,
it helps me in knowing which area i need to concentrate more for testing.
Comment 13 Hixie (not reading bugmail) 2001-11-01 04:07:06 PST
So, er, why no nsIDOMHTMLSpanElement?

Comment 14 Fabian Guisset 2001-11-01 07:24:19 PST
Now that I think of it... doesn't this happen to all DOM classes whose
mProtoChainChainInterface interface is nsIDOMHTMLElement?
namely HTMLDelElement, HTMLInsElement, HTMLSpacerElement, HTMLUnknownElement
(not sure this one should), HTMLWBRElement.
Is there a global solution to fix them all at once?
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2001-11-01 07:50:39 PST
> So, er, why no nsIDOMHTMLSpanElement?

Because it's not in DOM2 HTML.  See and
the table of contents of (which does not
list HTMLSpanElement)
Comment 16 Johnny Stenback (:jst, 2001-11-01 22:03:23 PST
Fabian, yes, all those will be fixed.
Comment 17 Johnny Stenback (:jst, 2001-11-06 03:41:30 PST
Created attachment 56702 [details] [diff] [review]
Better fix.
Comment 18 Johnny Stenback (:jst, 2001-11-06 03:50:08 PST
The attached patch fixes this problem. The patch adds a mHasClassInterface to
nsDOMClassInfoData so every class knows if it's proto chain interface is
specific to that class or not. For those classes where we don't have a class
interface we need to use the name of the proto chain interface for setting up
ctor.prototype.__proto__, whereas in the case of a class that *does* have a
class interface we use the parent of the proto chain interface when setting up

So in the case where we wrap a span element, our proto chain interface is
nsIDOMHTMLElement (since there is no nsIDOMHTMLSpanElement),
nsDOMClassInfo::PostCreate() ends up resolving "HTMLSpanElement" (before the
patch it incorrectly ended up resolving "HTMLElement" so the HTMLSpanElement
ctor was never properly set up). When resolving "HTMLSpanElement" we'll see that
the class doesn't have a class interface so we use the proto chain interface
nsIDOMHTMLElement directly (in stead of using its parent nsIDOMElement) for
figuring out what to set HTMLSpanElement.prototype.__proto__ to.

Oh, and storing mScriptableFlags in 31 bits is ok, see nsIXPCScriptable::RESERVED.
Comment 19 Johnny Stenback (:jst, 2001-11-06 04:10:13 PST
Actually the patch doesn't add mHasClassInterface, that was added by mistake in
a checkin unrelated to this bug. That's why you don't see the code that
specifies what classes don't have a class interface in this patch.

Here's the patch that was checked in earlier by mistake:
Comment 20 Fabian Guisset 2001-11-06 04:43:22 PST
Comment on attachment 56702 [details] [diff] [review]
Better fix.

r=fabian after jst fixed 
to be
Comment 21 Peter Van der Beken [:peterv] 2001-11-06 05:02:30 PST
Comment on attachment 56702 [details] [diff] [review]
Better fix.

Comment 22 John Bandhauer 2001-11-06 19:31:07 PST
Comment on attachment 56702 [details] [diff] [review]
Better fix.

Comment 23 Johnny Stenback (:jst, 2001-11-06 23:53:33 PST
Created attachment 56860 [details] [diff] [review]
Same as above, but even better.
Comment 24 Johnny Stenback (:jst, 2001-11-07 00:04:45 PST
So I had to tweak this a bit before checking in since I found one more minor
problem, I found more classes that had class interfaces (in theory, at least)
but whose name (due to conflicts with other class names) didn't match the name
of the proto chain interface. I also managed to eliminate a nsIID copy, so I did
that while I was poking around in this code. With this patch,
ComputedCSSStyleDeclaration.prototype.__proto__ ===
CSSStyleDeclaration.prototype, yay! That didn't work before this latest change.

The latest patch is checked in, if someone feels the need to do a post-checkin
review of the latest modifications, be my guest.

Oh, the latest patch is missing what Fabian pointed out, but the checked in
version did contain that fix.

Marking FIXED.
Comment 25 Fabian Guisset 2001-11-07 07:12:34 PST
Just wanted to let you know that there is now a warning that dbg_rv is not used.
And I'm afraid the compiler is right ;-)
Comment 26 Johnny Stenback (:jst, 2001-11-07 10:51:37 PST
Ah, that is true (in debug builds), I'll take out the unused variable...
Comment 27 Sivakiran Tummala 2002-03-08 14:34:39 PST
verified fixed

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