Changeset 239870 in webkit

Timestamp:

Jan 11, 2019 1:02:28 PM (5 years ago)

Author:

Matt Baker

Message:

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.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Views/TreeOutline.js (modified) (6 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js

@@ -330 +330 @@
         }
 
+        let removedIndexes = this._indexesForSubtree(child);
+
         if (child.previousSibling)
             child.previousSibling.nextSibling = child.nextSibling;
@@ -343 +345 @@
         child.previousSibling = null;
 
-        if (treeOutline)
+        if (treeOutline) {
+            treeOutline._selectionController.didRemoveItems(removedIndexes);
             treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementRemoved, {element: child});
+        }
     }
 
@@ -379 +383 @@
             }
 
+            let removedIndexes = treeOutline._indexesForSubtree(child);
+
             child._detach();
             child.treeOutline = null;
@@ -385 +391 @@
             child.previousSibling = null;
 
-            if (treeOutline)
+            this.children.shift();
+
+            if (treeOutline) {
+                treeOutline._selectionController.didRemoveItems(removedIndexes);
                 treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementRemoved, {element: child});
-
-            this.children.shift();
+            }
         }
     }
@@ -420 +428 @@
         }
         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--;
@@ -1073 +1077 @@
         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;
+    }
 };