Web Inspector: REGRESSION(r238599): Multiple Selection: restoring selection when...
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Dec 2018 23:16:25 +0000 (23:16 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Dec 2018 23:16:25 +0000 (23:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192091
<rdar://problem/46321795>

Reviewed by Devin Rousso.

* UserInterface/Controllers/SelectionController.js:
(WI.SelectionController.prototype.didInsertItem):
Fix a bug where selected indexes were overwritten by the inserted index.

* UserInterface/Views/TreeOutline.js:
(WI.TreeOutline):
(WI.TreeOutline.prototype.insertChild):
Update the SelectionController with the newly inserted index before
attaching the TreeElement. Attaching the TreeElement can cause it to
become selected, which would add the index to the SelectionController,
only to have it immediately incremented by the call to `didInsertItem`.
Additionally, change `insertionIndex` to be the index of the inserted
item instead of the inserted item's previous sibling.

(WI.TreeOutline.prototype._rememberTreeElement):
(WI.TreeOutline.prototype._forgetTreeElement):
(WI.TreeOutline.prototype._indexOfTreeElement.previousElement): Deleted.
Eliminate TreeElement index caching, which could become stale and cause
the wrong index to be calculated. Additionally, instead of walking up the
parent chain to determine the index, start at the root and use existing
method `traverseNextTreeElement`.

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
Source/WebInspectorUI/UserInterface/Views/TreeOutline.js

index 4e8fda3..d603a7e 100644 (file)
@@ -1,3 +1,33 @@
+2018-12-03  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: REGRESSION(r238599): Multiple Selection: restoring selection when opening WebInspector puts the TreeElement into a permanent selected state
+        https://bugs.webkit.org/show_bug.cgi?id=192091
+        <rdar://problem/46321795>
+
+        Reviewed by Devin Rousso.
+
+        * UserInterface/Controllers/SelectionController.js:
+        (WI.SelectionController.prototype.didInsertItem):
+        Fix a bug where selected indexes were overwritten by the inserted index.
+
+        * UserInterface/Views/TreeOutline.js:
+        (WI.TreeOutline):
+        (WI.TreeOutline.prototype.insertChild):
+        Update the SelectionController with the newly inserted index before
+        attaching the TreeElement. Attaching the TreeElement can cause it to
+        become selected, which would add the index to the SelectionController,
+        only to have it immediately incremented by the call to `didInsertItem`.
+        Additionally, change `insertionIndex` to be the index of the inserted
+        item instead of the inserted item's previous sibling.
+
+        (WI.TreeOutline.prototype._rememberTreeElement):
+        (WI.TreeOutline.prototype._forgetTreeElement):
+        (WI.TreeOutline.prototype._indexOfTreeElement.previousElement): Deleted.
+        Eliminate TreeElement index caching, which could become stale and cause
+        the wrong index to be calculated. Additionally, instead of walking up the
+        parent chain to determine the index, start at the root and use existing
+        method `traverseNextTreeElement`.
+
 2018-12-03  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Audit: test tree elements should start out collapsed
index 9456905..ea9e3bf 100644 (file)
@@ -203,8 +203,8 @@ WI.SelectionController = class SelectionController extends WI.Object
     {
         let current = this._selectedIndexes.lastIndex;
         while (current >= index) {
-            this._selectedIndexes.delete(index);
-            this._selectedIndexes.add(index + 1);
+            this._selectedIndexes.delete(current);
+            this._selectedIndexes.add(current + 1);
 
             current = this._selectedIndexes.indexLessThan(current);
         }
index 61a3c41..a7511d6 100644 (file)
@@ -56,7 +56,6 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
         this._cachedNumberOfDescendents = 0;
         this._selectionController = new WI.SelectionController(this);
-        this._treeElementIndexCache = new Map;
 
         this._itemWasSelectedByUser = false;
         this._processingSelectionChange = false;
@@ -295,6 +294,12 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         if (child.hasChildren && child.treeOutline._treeElementsExpandedState[child.identifier] !== undefined)
             child.expanded = child.treeOutline._treeElementsExpandedState[child.identifier];
 
+        // Update the SelectionController before attaching the TreeElement.
+        // Attaching the TreeElement can cause it to become selected, and
+        // added to the SelectionController.
+        let insertionIndex = this.treeOutline._indexOfTreeElement(child) || 0;
+        this.treeOutline._selectionController.didInsertItem(insertionIndex);
+
         if (this._childrenListNode)
             child._attach();
 
@@ -303,9 +308,6 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
         if (isFirstChild && this.expanded)
             this.expand();
-
-        let insertionIndex = this.treeOutline._indexOfTreeElement(child.previousSibling) || 0;
-        this.treeOutline._selectionController.didInsertItem(insertionIndex);
     }
 
     removeChildAtIndex(childIndex, suppressOnDeselect, suppressSelectSibling)
@@ -395,9 +397,6 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
     _rememberTreeElement(element)
     {
-        this._treeElementIndexCache.clear();
-        this._cachedNumberOfDescendents++;
-
         if (!this._knownTreeElements[element.identifier])
             this._knownTreeElements[element.identifier] = [];
 
@@ -408,19 +407,19 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
         // add the element
         elements.push(element);
+        this._cachedNumberOfDescendents++;
     }
 
     _forgetTreeElement(element)
     {
-        this._treeElementIndexCache.clear();
-        this._cachedNumberOfDescendents--;
-
         if (this.selectedTreeElement === element) {
             element.deselect(true);
             this.selectedTreeElement = null;
         }
-        if (this._knownTreeElements[element.identifier])
+        if (this._knownTreeElements[element.identifier]) {
             this._knownTreeElements[element.identifier].remove(element, true);
+            this._cachedNumberOfDescendents--;
+        }
     }
 
     _forgetChildrenRecursive(parentElement)
@@ -993,34 +992,22 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
     _indexOfTreeElement(treeElement)
     {
-        function previousElement(element) {
-            if (element.previousSibling) {
-                element = element.previousSibling;
-                if (element.children.length)
-                    element = element.children.lastValue;
-            } else
-                element = element.parent && element.parent.root ? null : element.parent;
-            return element;
-        }
+        const skipUnrevealed = false;
+        const stayWithin = null;
+        const dontPopulate = true;
 
         let index = 0;
-        let current = treeElement;
+        let current = this.children[0];
         while (current) {
-            let closestIndex = this._treeElementIndexCache.get(current);
-            if (!isNaN(closestIndex)) {
-                index += closestIndex;
-                break;
-            }
+            if (treeElement === current)
+                return index;
 
-            current = previousElement(current);
-            if (current)
-                index++;
+            current = current.traverseNextTreeElement(skipUnrevealed, stayWithin, dontPopulate);
+            ++index;
         }
 
-        if (!this._treeElementIndexCache.has(treeElement))
-            this._treeElementIndexCache.set(treeElement, index);
-
-        return index;
+        console.assert(false, "Unable to get index for tree element.", treeElement, treeOutline);
+        return NaN;
     }
 
     _treeElementAtIndex(index)