getAnonymousNodes returns null, not list, for a node with no binding

NEW
Unassigned

Status

()

Core
XBL
17 years ago
8 years ago

People

(Reporter: Vino Crescini, Unassigned)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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 &lt; anonChildArray.length; i++)
        dump("element = " + anonChildArray[i].nodeName + "\n");
      return;
    }
  </script>
  <box id="box1"/>
</window>
(Reporter)

Comment 1

17 years ago
Created attachment 24186 [details]
test file

Comment 2

17 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

17 years ago
Okay, I will really check it now (jrgm using trudelle's login).
Assignee: hyatt → jrgm
(Reporter)

Comment 4

17 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

17 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 6

17 years ago
easy workaround, can fix later, ->future
Target Milestone: --- → Future

Comment 7

16 years ago
Created attachment 60045 [details] [diff] [review]
"patch"

Comment 8

16 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

16 years ago
This is not good.  I'm marking this as wontfix.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → WONTFIX

Comment 10

16 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

16 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

16 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

16 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

16 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

16 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

16 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).
QA Contact: jrgmorrison → xbl

Updated

8 years ago
Assignee: hyatt → nobody
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
You need to log in before you can comment on or make changes to this bug.