609549 - inIDOMUtils needs getChildrenForNode to do what inDOMView does when a node is expanded to show children

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, enhancement)

Description

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: crashy getChildrenForNode

Firebug wants this, instead of the gotcha-filled nsIDOMDocumentXBL approach it's using.

Here's a patch that doesn't work.  It would take considerable hand-holding for me to see this through, since I have no idea what I'm doing regarding Gecko's smart pointers implementation.

Comment 1

[Boris Zbarsky [:bzbarsky]]

>+  *aChildren = kids;

  kids.forget(aChildren);

should do the trick, I'd think.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, and the point is I'm happy to hand-hold, review, explain why comment 1 makes sense, etc.

Comment 3

[Shawn Wilsher :sdwilsh]

Firebug needs this to work properly, correct?  Seeing as how this is an API change, this may need to block beta 7...

Comment 4

[Boris Zbarsky [:bzbarsky]]

We can add this to a separate interface post-b7 if it's a must-have, fwiw.

Comment 5

[LegNeato]

Not blocking b7 on it then. If for some reason we haven't built when this is ready, we'll likely take it....but not a blocker.

Comment 6

[John J. Barton]

(In reply to comment #3)
> Firebug needs this to work properly, correct? 

No, this would be for Chromebug, XUL level inspection.

Comment 7

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: forget the pointer to the outparam so the nsCOMPtr destructor doesn't try to free it

I guess this is the reason?

Comment 8

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: API-level test

Comment 9

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: API-level test with correct bug id

Comment 10

[Boris Zbarsky [:bzbarsky]]

> I guess this is the reason?

Yes.

The C++ code is fine now, but we do need that separate interface if we want this to land for Gecko 2.0....

Comment 11

[Colby Russell :crussell (no longer Mozilla-ing)]

(In reply to comment #10)
> > I guess this is the reason?
> 
> Yes.
> 
> The C++ code is fine now, but we do need that separate interface if we want
> this to land for Gecko 2.0....

That seems like a bad idea.  I'm fine with it going into 2.next.  Maybe someone else disagrees though.

Comment 12

[Boris Zbarsky [:bzbarsky]]

As long as we don't lose track of it, sure.  In that case, the patch looks fine except the interface needs a new uuid.

Comment 13

[Shawn Wilsher :sdwilsh]

Isn't in the case that we are allowing the change for *some* interfaces?

Also, we can add it as a new interface, and merge it post-2.0 (and make it all the same object in terms of JS now).

Comment 14

[Boris Zbarsky [:bzbarsky]]

> Isn't in the case that we are allowing the change for *some* interfaces?

I believe that at this point all interfaces are considered frozen unless the change is _really_ needed.

Comment 15

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: no bitrot

Comment 16

[Colby Russell :crussell (no longer Mozilla-ing)]

Does this still need a UUID change, since it was changed for bug 557726?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 530971 [details] [diff] [review]
no bitrot

Yes, this still needs an iid rev.  The rest looks fine.

Comment 18

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: New IID for inIDOMUtils

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 535733 [details] [diff] [review]
New IID for inIDOMUtils

r=me

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/projects/cedar/rev/d5d69f272f8f

Comment 21

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/d5d69f272f8f