Fix a regression where elements in subframes would not be revealed
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jul 2008 02:48:01 +0000 (02:48 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jul 2008 02:48:01 +0000 (02:48 +0000)
        or selected when inspected from the context menu. This was caused by
        JavaScript equality is not being true for JSInspectedObjectWrappers
        of the same node wrapped with different global ExecStates. This change
        adds a helper function that uses isSameNode to compare wrapped nodes.

        https://bugs.webkit.org/show_bug.cgi?id=19377

        Reviewed by Adam Roben.

        * page/inspector/ElementsPanel.js:
        (WebInspector.ElementsPanel.prototype.set rootDOMNode): Use objectsAreSame
        to compare nodes.
        (WebInspector.ElementsPanel.prototype.set focusedDOMNode): Ditto.
        (WebInspector.ElementsPanel.prototype.set hoveredDOMNode): Ditto.
        (WebInspector.ElementsPanel.prototype._updateModifiedNodes): Ditto.
        (WebInspector.ElementsPanel.prototype.revealNode): Ditto.
        (WebInspector.ElementsPanel.prototype.updateBreadcrumb): Ditto.
        (WebInspector.DOMNodeTreeElement.prototype.updateChildren): Ditto.
        * page/inspector/treeoutline.js:
        (TreeOutline.prototype.findTreeElement): Add an equal argument
        to accept a functions to compare two representedObjects. Defaults
        to strict equal if not supplied. All current clients pass objectsAreSame.
        * page/inspector/utilities.js:
        (Node.prototype.enclosingNodeOrSelfWithNodeNameInArray): Use objectsAreSame
        to compare nodes.
        (Node.prototype.enclosingNodeOrSelfWithClass): Ditto.
        (Element.prototype.query): Use the ownerDocument of the node, not document.
        (objectsAreSame): Added. Compares strict equal first, then uses isSameNode if
        it exists on both objects.
        (isAncestorNode): Use objectsAreSame to compare nodes.
        (firstCommonNodeAncestor): Ditto.
        (traverseNextNode): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@35315 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/page/inspector/ElementsPanel.js
WebCore/page/inspector/treeoutline.js
WebCore/page/inspector/utilities.js

index 1c02f1b..cd772b9 100644 (file)
@@ -1,3 +1,39 @@
+2008-07-22  Timothy Hatcher  <timothy@apple.com>
+
+        Fix a regression where elements in subframes would not be revealed
+        or selected when inspected from the context menu. This was caused by
+        JavaScript equality is not being true for JSInspectedObjectWrappers
+        of the same node wrapped with different global ExecStates. This change
+        adds a helper function that uses isSameNode to compare wrapped nodes.
+
+        https://bugs.webkit.org/show_bug.cgi?id=19377
+
+        Reviewed by Adam Roben.
+
+        * page/inspector/ElementsPanel.js:
+        (WebInspector.ElementsPanel.prototype.set rootDOMNode): Use objectsAreSame
+        to compare nodes.
+        (WebInspector.ElementsPanel.prototype.set focusedDOMNode): Ditto.
+        (WebInspector.ElementsPanel.prototype.set hoveredDOMNode): Ditto. 
+        (WebInspector.ElementsPanel.prototype._updateModifiedNodes): Ditto.
+        (WebInspector.ElementsPanel.prototype.revealNode): Ditto.
+        (WebInspector.ElementsPanel.prototype.updateBreadcrumb): Ditto.
+        (WebInspector.DOMNodeTreeElement.prototype.updateChildren): Ditto.
+        * page/inspector/treeoutline.js:
+        (TreeOutline.prototype.findTreeElement): Add an equal argument
+        to accept a functions to compare two representedObjects. Defaults
+        to strict equal if not supplied. All current clients pass objectsAreSame.
+        * page/inspector/utilities.js:
+        (Node.prototype.enclosingNodeOrSelfWithNodeNameInArray): Use objectsAreSame
+        to compare nodes.
+        (Node.prototype.enclosingNodeOrSelfWithClass): Ditto.
+        (Element.prototype.query): Use the ownerDocument of the node, not document.
+        (objectsAreSame): Added. Compares strict equal first, then uses isSameNode if
+        it exists on both objects.
+        (isAncestorNode): Use objectsAreSame to compare nodes.
+        (firstCommonNodeAncestor): Ditto.
+        (traverseNextNode): Ditto.
+
 2008-07-21  Timothy Hatcher  <timothy@apple.com>
 
         Added InspectorController.isWindowVisible to the JavaScript class
index 6bf1407..270032e 100644 (file)
@@ -172,7 +172,7 @@ WebInspector.ElementsPanel.prototype = {
 
     set rootDOMNode(x)
     {
-        if (this._rootDOMNode === x)
+        if (objectsAreSame(this._rootDOMNode, x))
             return;
 
         this._rootDOMNode = x;
@@ -188,7 +188,7 @@ WebInspector.ElementsPanel.prototype = {
 
     set focusedDOMNode(x)
     {
-        if (this._focusedDOMNode === x) {
+        if (objectsAreSame(this._focusedDOMNode, x)) {
             var nodeItem = this.revealNode(x);
             if (nodeItem)
                 nodeItem.select();
@@ -211,7 +211,7 @@ WebInspector.ElementsPanel.prototype = {
 
     set hoveredDOMNode(x)
     {
-        if (this._hoveredDOMNode === x)
+        if (objectsAreSame(this._hoveredDOMNode, x))
             return;
 
         this._hoveredDOMNode = x;
@@ -292,7 +292,7 @@ WebInspector.ElementsPanel.prototype = {
 
     revealNode: function(node)
     {
-        var nodeItem = this.treeOutline.findTreeElement(node, this._isAncestorIncludingParentFrames.bind(this), this._parentNodeOrFrameElement.bind(this));
+        var nodeItem = this.treeOutline.findTreeElement(node, this._isAncestorIncludingParentFrames.bind(this), this._parentNodeOrFrameElement.bind(this), objectsAreSame);
         if (!nodeItem)
             return;
 
@@ -328,7 +328,7 @@ WebInspector.ElementsPanel.prototype = {
         var foundRoot = false;
         var crumb = crumbs.firstChild;
         while (crumb) {
-            if (crumb.representedObject === this.rootDOMNode)
+            if (objectsAreSame(crumb.representedObject, this.rootDOMNode))
                 foundRoot = true;
 
             if (foundRoot)
@@ -336,7 +336,7 @@ WebInspector.ElementsPanel.prototype = {
             else
                 crumb.removeStyleClass("dimmed");
 
-            if (crumb.representedObject === this.focusedDOMNode) {
+            if (objectsAreSame(crumb.representedObject, this.focusedDOMNode)) {
                 crumb.addStyleClass("selected");
                 handled = true;
             } else {
@@ -428,7 +428,7 @@ WebInspector.ElementsPanel.prototype = {
             if (current.nodeType === Node.DOCUMENT_NODE)
                 continue;
 
-            if (current === this.rootDOMNode)
+            if (objectsAreSame(current, this.rootDOMNode))
                 foundRoot = true;
 
             var crumb = document.createElement("span");
@@ -513,7 +513,7 @@ WebInspector.ElementsPanel.prototype = {
 
             if (foundRoot)
                 crumb.addStyleClass("dimmed");
-            if (current === this.focusedDOMNode)
+            if (objectsAreSame(current, this.focusedDOMNode))
                 crumb.addStyleClass("selected");
             if (!crumbs.childNodes.length)
                 crumb.addStyleClass("end");
index 163e357..5c10cf1 100644 (file)
@@ -245,18 +245,21 @@ TreeOutline.prototype._forgetTreeElement = function(element)
     }
 }
 
-TreeOutline.prototype.findTreeElement = function(representedObject, isAncestor, getParent)
+TreeOutline.prototype.findTreeElement = function(representedObject, isAncestor, getParent, equal)
 {
     if (!representedObject)
         return null;
 
+    if (!equal)
+        equal = function(a, b) { return a === b };
+
     if ("__treeElementIdentifier" in representedObject) {
         // If this representedObject has a tree element identifier, and it is a known TreeElement
         // in our tree we can just return that tree element.
         var elements = this._knownTreeElements[representedObject.__treeElementIdentifier];
         if (elements) {
             for (var i = 0; i < elements.length; ++i)
-                if (elements[i].representedObject === representedObject)
+                if (equal(elements[i].representedObject, representedObject))
                     return elements[i];
         }
     }
@@ -270,7 +273,7 @@ TreeOutline.prototype.findTreeElement = function(representedObject, isAncestor,
     var found = false;
     for (var i = 0; i < this.children.length; ++i) {
         item = this.children[i];
-        if (item.representedObject === representedObject || isAncestor(item.representedObject, representedObject)) {
+        if (equal(item.representedObject, representedObject) || isAncestor(item.representedObject, representedObject)) {
             found = true;
             break;
         }
@@ -285,7 +288,7 @@ TreeOutline.prototype.findTreeElement = function(representedObject, isAncestor,
     var currentObject = representedObject;
     while (currentObject) {
         ancestors.unshift(currentObject);
-        if (currentObject === item.representedObject)
+        if (equal(currentObject, item.representedObject))
             break;
         currentObject = getParent(currentObject);
     }
@@ -294,18 +297,18 @@ TreeOutline.prototype.findTreeElement = function(representedObject, isAncestor,
     for (var i = 0; i < ancestors.length; ++i) {
         // Make sure we don't call findTreeElement with the same representedObject
         // again, to prevent infinite recursion.
-        if (ancestors[i] === representedObject)
+        if (equal(ancestors[i], representedObject))
             continue;
         // FIXME: we could do something faster than findTreeElement since we will know the next
         // ancestor exists in the tree.
-        item = this.findTreeElement(ancestors[i], isAncestor, getParent);
+        item = this.findTreeElement(ancestors[i], isAncestor, getParent, equal);
         if (item && item.onpopulate)
             item.onpopulate(item);
     }
 
     // Now that all the ancestors are populated, try to find the representedObject again. This time
     // without the isAncestor and getParent functions to prevent an infinite recursion if it isn't found.
-    return this.findTreeElement(representedObject);
+    return this.findTreeElement(representedObject, null, null, equal);
 }
 
 TreeOutline.prototype.treeElementFromPoint = function(x, y)
index 7e87413..b3815fd 100644 (file)
@@ -135,7 +135,7 @@ Element.prototype.hasStyleClass = function(className)
 
 Node.prototype.enclosingNodeOrSelfWithNodeNameInArray = function(nameArray)
 {
-    for (var node = this; node && (node !== document); node = node.parentNode)
+    for (var node = this; node && !objectsAreSame(node, this.ownerDocument); node = node.parentNode)
         for (var i = 0; i < nameArray.length; ++i)
             if (node.nodeName.toLowerCase() === nameArray[i].toLowerCase())
                 return node;
@@ -149,7 +149,7 @@ Node.prototype.enclosingNodeOrSelfWithNodeName = function(nodeName)
 
 Node.prototype.enclosingNodeOrSelfWithClass = function(className)
 {
-    for (var node = this; node && (node !== document); node = node.parentNode)
+    for (var node = this; node && !objectsAreSame(node, this.ownerDocument); node = node.parentNode)
         if (node.nodeType === Node.ELEMENT_NODE && node.hasStyleClass(className))
             return node;
     return null;
@@ -164,7 +164,7 @@ Node.prototype.enclosingNodeWithClass = function(className)
 
 Element.prototype.query = function(query) 
 {
-    return document.evaluate(query, this, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
+    return this.ownerDocument.evaluate(query, this, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
 }
 
 Element.prototype.removeChildren = function()
@@ -491,6 +491,21 @@ function nodeContentPreview()
     return preview.collapseWhitespace();
 }
 
+function objectsAreSame(a, b)
+{
+    // FIXME: Make this more generic so is works with any wrapped object, not just nodes.
+    // This function is used to compare nodes that might be JSInspectedObjectWrappers, since
+    // JavaScript equality is not true for JSInspectedObjectWrappers of the same node wrapped
+    // with different global ExecStates, we use isSameNode to compare them.
+    if (a === b)
+        return true;
+    if (!a || !b)
+        return false;
+    if (a.isSameNode && b.isSameNode)
+        return a.isSameNode(b);
+    return false;
+}
+
 function isAncestorNode(ancestor)
 {
     if (!this || !ancestor)
@@ -498,7 +513,7 @@ function isAncestorNode(ancestor)
 
     var currentNode = ancestor.parentNode;
     while (currentNode) {
-        if (this === currentNode)
+        if (objectsAreSame(this, currentNode))
             return true;
         currentNode = currentNode.parentNode;
     }
@@ -519,13 +534,13 @@ function firstCommonNodeAncestor(node)
     var node1 = this.parentNode;
     var node2 = node.parentNode;
 
-    if ((!node1 || !node2) || node1 !== node2)
+    if ((!node1 || !node2) || !objectsAreSame(node1, node2))
         return null;
 
     while (node1 && node2) {
         if (!node1.parentNode || !node2.parentNode)
             break;
-        if (node1 !== node2)
+        if (!objectsAreSame(node1, node2))
             break;
 
         node1 = node1.parentNode;
@@ -584,7 +599,7 @@ function traverseNextNode(skipWhitespace, stayWithin)
     if (node)
         return node;
 
-    if (stayWithin && this === stayWithin)
+    if (stayWithin && objectsAreSame(this, stayWithin))
         return null;
 
     node = skipWhitespace ? nextSiblingSkippingWhitespace.call(this) : this.nextSibling;
@@ -592,7 +607,7 @@ function traverseNextNode(skipWhitespace, stayWithin)
         return node;
 
     node = this;
-    while (node && !(skipWhitespace ? nextSiblingSkippingWhitespace.call(node) : node.nextSibling) && (!stayWithin || !node.parentNode || node.parentNode !== stayWithin))
+    while (node && !(skipWhitespace ? nextSiblingSkippingWhitespace.call(node) : node.nextSibling) && (!stayWithin || !node.parentNode || !objectsAreSame(node.parentNode, stayWithin)))
         node = node.parentNode;
     if (!node)
         return null;