664184 - Mistakes in JavaScript Object viewer implementation

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

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

When fixing but 193942, I noticed three things in the new jsObject tree view:

1.  isContainerEmpty begins as follows:

>   if (!this.isContainer(aIndex)) {
>     return true;
>   }

This should, of course, return false.  This has the effect that after the subject setter calls toggleOpenState, if the subject is a string, the row is expanded to show the string's indexes.

2. JSOV_ToggleOpenState doesn't need to call checkForBadIndex.  JSOV_IsContainerEmpty will call isContainer, which will in turn call checkForBadIndex, and if you follow along as the stack then unwinds, you'll notice that it ends up doing the Right Thing.

3. When comparing JSOV_IsContainer to InspectInNewWindowBase_IsInspectable, I noticed that the former doesn't handle null correctly.  In fact, baseCommands.js is already in use, so after we check for a bad index, we can just return the result of a call to the latter.

Comment 1

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

Attachment: fix the JSOV_IsContainerEmpty return value; don't call checkForBadIndex in JSOV_ToggleOpenState; call isInspectable in JSOV_IsContainer

(In reply to comment #0)
> 1.  isContainerEmpty begins as follows:
> 
> >   if (!this.isContainer(aIndex)) {
> >     return true;
> >   }
> 
> This should, of course, return false.

Er, the other way around, I mean.  false is what it returns now, true is what it should return.

Comment 2

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

Pushed: http://hg.mozilla.org/dom-inspector/rev/697016a1e944