Coding error in jsObject.js

RESOLVED FIXED

Status

Other Applications
DOM Inspector
RESOLVED FIXED
14 years ago
11 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

1005 bytes, patch
timeless
: review+
Christopher Aillon (sabbatical, not receiving bugmail)
: review+
Details | Diff | Splinter Review
813 bytes, patch
timeless
: review+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
 
(Assignee)

Comment 1

14 years ago
Created attachment 133189 [details] [diff] [review]
Proposed patch

So, was kids supposed to be a node or a node list?
Comment on attachment 133189 [details] [diff] [review]
Proposed patch

>   openTreeItem: function(aItem)
>   {
>-    var kids = aItem.getElementsByTagName("treechildren");
>-    if (kids.length == 0) {
>-      kids = document.createElement("treechildren");
>+    var treechildren = aItem.getElementsByTagName("treechildren").item(0);
>+    if (!treechildren) {
>+      treechildren = document.createElement("treechildren");

A rose by any other name... the intent of the kids argument is pretty clear
here, I think.

>       aItem.appendChild(kids);

And if you really want this, this argument needs to change too, or you get an
instant strict JS warning and a regression, at the very least.

>     }
>     
>-    this.buildPropertyTree(kids, aItem.__JSValue__);
>+    this.buildPropertyTree(treechildren, aItem.__JSValue__);

Ditto on the rose.

>   },
>   
>   onCreateContext: function(aPopup)
>@@ -276,7 +276,7 @@ function getSelectedItem()
> {
>   var tree = document.getElementById("treeJSObject");
>   if (tree.view.selection.count)
>-    return tree.contentView.getItemAtIndex(tree.curentIndex);
>+    return tree.view.getItemAtIndex(tree.curentIndex);

tree.currentIndex

Finally, I'm working on revising my patch to bug 193942, where half of the
jsObject.js file is rewritten.	The openTreeItem() function will be obsoleted
and removed by my patch.  (I know it's taking me forever, but I think you
should consider the revisions.)
(Assignee)

Comment 3

14 years ago
As you noticed I attached the wrong diff...

Anyway IMHO "kids" is an inappropriate name for an element.
Product: Core → Other Applications
(Assignee)

Comment 4

12 years ago
*** Bug 317322 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 5

12 years ago
Created attachment 203931 [details] [diff] [review]
Corrected patch
Assignee: dom-inspector → neil.parkwaycc.co.uk
Attachment #133189 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #203931 - Flags: review?(timeless)

Updated

12 years ago
Attachment #203931 - Flags: review?(timeless) → review+
(Assignee)

Comment 6

12 years ago
Comment on attachment 203931 [details] [diff] [review]
Corrected patch

Given as there's no moa? flag in bugzilla...
Attachment #203931 - Flags: review?(caillon)
Attachment #203931 - Flags: review?(caillon) → review+
(Assignee)

Comment 7

12 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

12 years ago
Oops, fix is wrong, coding error hid logic error.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

12 years ago
Created attachment 223165 [details] [diff] [review]
Logic fix
Attachment #223165 - Flags: review?(timeless)

Updated

12 years ago
Attachment #223165 - Flags: review?(timeless) → review+
(Assignee)

Comment 10

12 years ago
Fix for logic error hidden by coding error checked in.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
QA Contact: timeless → dom-inspector
You need to log in before you can comment on or make changes to this bug.