Title: [238825] trunk/Source/WebInspectorUI
- Revision
- 238825
- Author
- [email protected]
- Date
- 2018-12-03 15:16:25 -0800 (Mon, 03 Dec 2018)
Log Message
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`.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (238824 => 238825)
--- trunk/Source/WebInspectorUI/ChangeLog 2018-12-03 22:52:14 UTC (rev 238824)
+++ trunk/Source/WebInspectorUI/ChangeLog 2018-12-03 23:16:25 UTC (rev 238825)
@@ -1,3 +1,33 @@
+2018-12-03 Matt Baker <[email protected]>
+
+ 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 <[email protected]>
Web Inspector: Audit: test tree elements should start out collapsed
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js (238824 => 238825)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js 2018-12-03 22:52:14 UTC (rev 238824)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js 2018-12-03 23:16:25 UTC (rev 238825)
@@ -203,8 +203,8 @@
{
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);
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js (238824 => 238825)
--- trunk/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js 2018-12-03 22:52:14 UTC (rev 238824)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js 2018-12-03 23:16:25 UTC (rev 238825)
@@ -56,7 +56,6 @@
this._cachedNumberOfDescendents = 0;
this._selectionController = new WI.SelectionController(this);
- this._treeElementIndexCache = new Map;
this._itemWasSelectedByUser = false;
this._processingSelectionChange = false;
@@ -295,6 +294,12 @@
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 @@
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 @@
_rememberTreeElement(element)
{
- this._treeElementIndexCache.clear();
- this._cachedNumberOfDescendents++;
-
if (!this._knownTreeElements[element.identifier])
this._knownTreeElements[element.identifier] = [];
@@ -408,19 +407,19 @@
// 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 @@
_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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
