579339 - Improve JS/DOM Object Introspector

Status: RESOLVED DUPLICATE of bug 573102

(DevTools :: General, defect)

Description

[Julian Viereck]

This is a following bug for 573102. PropertyPanel didn't worked before for the 
window or document object. Arrays can now be inspected. Few other adjustments.

Comment 1

[Julian Viereck]

Attachment: Patch

Comment 2

[Julian Viereck]

Attachment: Patch rebased

Same as former patch attachment 457856 [details] [diff] [review], but rebased.

Comment 3

[Julian Viereck]

Attachment: Patch rebase 2

Rebase with trunk.

Also, add a new JSTerm evalInSandbox() function as a result of bug 574033. Improved the unit test a little bit.

Comment 4

[Julian Viereck]

I'm requesting 2.0 blocking status for this bug as the added patch improves an important feature that is required by the WebConsole (aka Heads Up Display) and DOM Inspector.

Comment 5

[Julian Viereck]

Attachment: Patch 1.1

Same as 458358 but without two Services.console.logStringMessage(...args); statements.

Comment 6

[David Dahl :ddahl]

Comment on attachment 459338 [details] [diff] [review]
Patch 1.1


>+function unwrapObject(aObject)
> {
>-  if (!aObject) {
>-    return false;
>+  if (typeof(aObject) === "undefined" || aObject == null) {
>+    return aObject;
is this supposed to actually unwrap an object? You can also do: return XPCNativeWrapper.unwrap(aObject)

>-  // DISCUSS: Why not showing the body of the function as well?
perhaps because it is deep in chrome code so you will be shown "[Native Code]" or something like that.


>+/**
>+ * Get an array of property name value pairs for the tree.
>  *
>  * @param object aObject
>  *        The object to get properties for.
>  * @returns array of object
>- *          Objects have the name: and value: properties.
>+ *          Objects have the name, value, display, type, children properties.
>  */
> function namesAndValuesOf(aObject)
> {
>   let pairs = [];
>+  let value, presentable;
> 
>-  for (var name in aObject) {
>+  for (var propName in aObject) {
>+    try {
>+      value = aObject[propName];
>+      presentable = presentableValueFor(value);
>+    }
>+    catch (ex) {
>+      continue;
>+    }
>+
>     let pair = {};
>-    pair.name = name;
>-    pair.display = name + ': ' + presentableValueFor(aObject[name], name);
>-    pair.value = aObject[name];
>+    pair.name = propName;
>+    pair.display = propName + ": " + presentable.display;
>+    pair.type = presentable.type;
>+    pair.value = value;
>+
>+    // Convert the pair.name to a number for later sorting.
>+    pair.nameNumber = parseFloat(pair.name)
>+    if (isNaN(pair.nameNumber)) {
>+      pair.nameNumber = false;
>+    }
> 
>     pairs.push(pair);
>   }
> 
>   pairs.sort(function(a, b)
>   {
>-    if (a.name < b.name) {
>+    // Sort numbers.
>+    if (a.nameNumber !== false && b.nameNumber === false) {
>+      return 1;
>+    }
>+    else if (a.nameNumber === false && b.nameNumber !== false) {
>       return -1;
>     }
>-    if (a.name > b.name) {
>+    else if (a.nameNumber !== false && b.nameNumber !== false) {
>+      return a.nameNumber - b.nameNumber;
>+    }
>+    // Sort string.
>+    else if (a.name < b.name) {
>+      return -1;
>+    }
>+    else if (a.name > b.name) {
>       return 1;
>     }
>-    return 0;
>+    else {
>+      return 0;
>+    }
>   });
> 

Have you tested this sorting on some objects with many properties of varying types?

>   return pairs;
> }
>

Comment 7

[Julian Viereck]

Attachment: Patch 1.2

Updated patch:
- tuned sorting algorithm to put numbers infront of characters
- added testcase for property sorting
- removed unwrap() function. I tested this on the window/document object + some other JS objects and I could see no regression when removing them.

Comment 8

[Johnathan Nightingale [:johnath]]

I'd like to block once on either this bug or bug 573102. This was filed as a follow up to bug 573102, but it hasn't landed yet - is merging them into one bug a bad idea for some reason?

Comment 9

[David Dahl :ddahl]

(In reply to comment #8)
> I'd like to block once on either this bug or bug 573102. This was filed as a
> follow up to bug 573102, but it hasn't landed yet - is merging them into one
> bug a bad idea for some reason?

No, it is not a bad idea. two patches on one bug. yay!

Comment 10

[David Dahl :ddahl]

Comment on attachment 460207 [details] [diff] [review]
Patch 1.2

looks good, nice test.

Comment 11

[Julian Viereck]

The patch of this bug has been merged in bug 573102. Mark this as duplicate to not confuse people as work/review is done on that bug now.