Web Inspector: REGRESSION: deleting an audit puts selection in a selected but invisib...

https://bugs.webkit.org/show_bug.cgi?id=192917
<rdar://problem/46875285>

Reviewed by Devin Rousso.

SelectionController should not be notified of removed children until the
child items have been removed from the TreeOutline. Doing so at this stage
is unsafe, since this method checks `this.selectedTreeElement`, which could
return the adjusted index from the SelectionController before anything has
actually been removed from the TreeOutline.

The number of calls to SelectionController.prototype.didRemoveItems is also
reduced somewhat, since we're no longer calling it for every TreeElement.

* UserInterface/Views/TreeOutline.js:
(WI.TreeOutline.prototype.removeChildAtIndex):
(WI.TreeOutline.prototype.removeChildren):
(WI.TreeOutline.prototype._forgetTreeElement):
(WI.TreeOutline.prototype._indexesForSubtree): Added.

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

diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
index 819fc0b..5f28824 100644 (file)
--- a/Source/WebInspectorUI/ChangeLog
+++ b/Source/WebInspectorUI/ChangeLog
@@ -1,3 +1,26 @@
+2019-01-11  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: REGRESSION: deleting an audit puts selection in a selected but invisible state
+        https://bugs.webkit.org/show_bug.cgi?id=192917
+        <rdar://problem/46875285>
+
+        Reviewed by Devin Rousso.
+
+        SelectionController should not be notified of removed children until the
+        child items have been removed from the TreeOutline. Doing so at this stage
+        is unsafe, since this method checks `this.selectedTreeElement`, which could
+        return the adjusted index from the SelectionController before anything has
+        actually been removed from the TreeOutline.
+
+        The number of calls to SelectionController.prototype.didRemoveItems is also
+        reduced somewhat, since we're no longer calling it for every TreeElement.
+
+        * UserInterface/Views/TreeOutline.js:
+        (WI.TreeOutline.prototype.removeChildAtIndex):
+        (WI.TreeOutline.prototype.removeChildren):
+        (WI.TreeOutline.prototype._forgetTreeElement):
+        (WI.TreeOutline.prototype._indexesForSubtree): Added.        
+
 2019-01-10  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Audit: allow audits to be enabled/disabled

diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
index 22840a1..9a0183e 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
@@ -329,6 +329,8 @@ WI.TreeOutline = class TreeOutline extends WI.Object
             treeOutline._forgetChildrenRecursive(child);
         }
 
+        let removedIndexes = this._indexesForSubtree(child);
+
         if (child.previousSibling)
             child.previousSibling.nextSibling = child.nextSibling;
         if (child.nextSibling)
@@ -342,8 +344,10 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         child.nextSibling = null;
         child.previousSibling = null;
 
-        if (treeOutline)
+        if (treeOutline) {
+            treeOutline._selectionController.didRemoveItems(removedIndexes);
             treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementRemoved, {element: child});
+        }
     }
 
     removeChild(child, suppressOnDeselect, suppressSelectSibling)
@@ -378,16 +382,20 @@ WI.TreeOutline = class TreeOutline extends WI.Object
                 treeOutline._forgetChildrenRecursive(child);
             }
 
+            let removedIndexes = treeOutline._indexesForSubtree(child);
+
             child._detach();
             child.treeOutline = null;
             child.parent = null;
             child.nextSibling = null;
             child.previousSibling = null;
 
-            if (treeOutline)
-                treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementRemoved, {element: child});
-
             this.children.shift();
+
+            if (treeOutline) {
+                treeOutline._selectionController.didRemoveItems(removedIndexes);
+                treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementRemoved, {element: child});
+            }
         }
     }
 
@@ -419,10 +427,6 @@ WI.TreeOutline = class TreeOutline extends WI.Object
             this.selectedTreeElement = null;
         }
         if (this._knownTreeElements[element.identifier]) {
-            let index = this._indexOfTreeElement(element);
-            if (index >= 0)
-                this._selectionController.didRemoveItems(new WI.IndexSet([index]));
-
             this._knownTreeElements[element.identifier].remove(element, true);
             this._cachedNumberOfDescendents--;
         }
@@ -1072,6 +1076,39 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
         this.dispatchEventToListeners(WI.TreeOutline.Event.SelectionDidChange, {selectedByUser});
     }
+
+    _indexesForSubtree(treeElement)
+    {
+        let treeOutline = treeElement.treeOutline;
+        if (!treeOutline)
+            return new WI.IndexSet;
+
+        let firstChild = treeElement.children[0];
+        if (!firstChild)
+            return new WI.IndexSet;
+
+        let startIndex = treeOutline._indexOfTreeElement(firstChild);
+        let endIndex = startIndex;
+
+        const skipUnrevealed = false;
+        const stayWithin = treeElement;
+        const dontPopulate = true;
+
+        let current = firstChild;
+        while (current = current.traverseNextTreeElement(skipUnrevealed, stayWithin, dontPopulate))
+            endIndex++;
+
+        // Include the index of the subtree's root, unless it's the TreeOutline root.
+        if (!treeElement.root)
+            startIndex--;
+
+        let count = endIndex - startIndex + 1;
+
+        let indexes = new WI.IndexSet;
+        indexes.addRange(startIndex, count);
+
+        return indexes;
+    }
 };
 
 WI.TreeOutline._styleElement = null;