Closed
Bug 67384
Opened 24 years ago
Closed 5 years ago
getAnonymousNodes returns null, not list, for a node with no binding
Categories
(Core :: XBL, defect)
Core
XBL
Tracking
()
RESOLVED
WONTFIX
Future
People
(Reporter: vino, Unassigned)
Details
Attachments
(2 files)
564 bytes,
text/xml
|
Details | |
973 bytes,
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.12-20 i586; en-US; 0.7) Gecko/20010105 BuildID: 2001010517 in JS, a call to the function document.getAnonymousNodes(somenode) where somenode is a xul element with no anonymous content will cause a crash.Shouldn't it simply return an empty list? Reproducible: Always Steps to Reproduce: run the xul file below Actual Results: release build: abnormal exit debug build: ###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../../dist/include/nsCOMPtr.h, line 648 ###!!! Break: at file ../../../../dist/include/nsCOMPtr.h, line 648 Expected Results: getAnonymousNodes() should have returned an array of length 0 <?xml version="1.0"?> <window xmlns:html="http://www.w3.org/TR/REC-html40" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" width="100" height="100" onload="start();"> <script language="javascript"> function start() { var boxElement = document.getElementById("box1"); var anonChildArray = document.getAnonymousNodes(boxElement); for (var i = 0; i < anonChildArray.length; i++) dump("element = " + anonChildArray[i].nodeName + "\n"); return; } </script> <box id="box1"/> </window>
Reporter | ||
Comment 1•24 years ago
|
||
Comment 2•24 years ago
|
||
Well, no sooner does one find a bug than hyatt decides to rewrite great reams of code. This was crashing in the now-deceased nsXBLService::GetContentList(). I'll see if this condition/crash is fixed in the new code tomorrow. Thanks for the nice test case.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•24 years ago
|
||
Okay, I will really check it now (jrgm using trudelle's login).
Assignee: hyatt → jrgm
Reporter | ||
Comment 4•24 years ago
|
||
I just tried it on 0.8. doesn't cause a crash anymore but it returns a null when there are no anonymous children. shouldn't it return an empty array instead?
Comment 5•24 years ago
|
||
Right you are. No crash, but that method should return the empty list, rather than |null| to be consistent with other DOM methods.
Assignee: jrgm → hyatt
OS: Linux → All
Hardware: PC → All
Summary: crash when trying to retrieve an anonymous content from a node with no binding → getAnonymousNodes returns null, not list, for a node with no binding
Comment 7•23 years ago
|
||
Comment 8•23 years ago
|
||
This "patch" fixes the problem by doing what we would have done in nsXULElement::GetChildNodes if box had a binding. I can't say I really understand why this is necessary. Hoping David "xpconnect" Bradley can explain.
Comment 9•23 years ago
|
||
This is not good. I'm marking this as wontfix.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WONTFIX
Comment 10•23 years ago
|
||
I'm not saying that there is any pressing need to fix this, but I think this is still a valid bug. DOM methods such as getElementsByTagName() and childNodes() always return a NodeList, which is a, possibly zero-length, list. They don't return null when they have no members. I don't see why it's preferable to have getAnonymousNodes() not return an empty list.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 11•23 years ago
|
||
Well, then we'd have to make a cheesy internal method that did return null just to deal with this. The problem is this method is called all the time as you walk down a frame tree, and you'll end up allocating all of these node lists for the frames that don't have anonymous content. I would guess that Blake's patch would spike page load times by 5-10%.
Comment 12•23 years ago
|
||
Anyway, the binding manager method should return null. I'll accept a patch to DocumentXBL's method to make a nodelist, but the binding manager method can't be changed.
Status: REOPENED → ASSIGNED
Comment 13•23 years ago
|
||
> > I don't see why it's preferable to ...
>
> ... would spike page load times by 5-10% ...
Ok, now I see :-].
Comment 14•23 years ago
|
||
Yes, I didn't expect to check this in, hence "patch"; I notice we hit the !binding case many, many times.
Comment 15•23 years ago
|
||
I'm still confused though about how this method is just returning an object, though, and not a nodelist (as shown by alert(typeof retval)), given http://lxr.mozilla.org/seamonkey/source/dom/public/idl/xbl/nsIDOMDocumentXBL.idl#46 Can someone explain that to me, just for my own knowledge?
Comment 16•23 years ago
|
||
blake: try alert(retval) alert(typeof window) will yield 'object' too. It is not "just an object", but it *is* an object (not a string, number, function, or undefined).
Updated•15 years ago
|
QA Contact: jrgmorrison → xbl
Updated•15 years ago
|
Assignee: hyatt → nobody
Comment 17•14 years ago
|
||
This is a mass change. Every comment has "assigned-to-new" in it. I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.
Status: ASSIGNED → NEW
Comment 18•5 years ago
|
||
XBL is now disabled in Firefox (Bug 1583314) and is in the process of being removed from Gecko (Bug 1566221), so closing bugs requesting changes to its implementation as wontfix.
Status: NEW → RESOLVED
Closed: 23 years ago → 5 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•