Changeset 238825 in webkit


Ignore:
Timestamp:
Dec 3, 2018 3:16:25 PM (5 years ago)
Author:
Matt Baker
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.

Location:
trunk/Source/WebInspectorUI
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebInspectorUI/ChangeLog

    r238822 r238825  
     12018-12-03  Matt Baker  <mattbaker@apple.com>
     2
     3        Web Inspector: REGRESSION(r238599): Multiple Selection: restoring selection when opening WebInspector puts the TreeElement into a permanent selected state
     4        https://bugs.webkit.org/show_bug.cgi?id=192091
     5        <rdar://problem/46321795>
     6
     7        Reviewed by Devin Rousso.
     8
     9        * UserInterface/Controllers/SelectionController.js:
     10        (WI.SelectionController.prototype.didInsertItem):
     11        Fix a bug where selected indexes were overwritten by the inserted index.
     12
     13        * UserInterface/Views/TreeOutline.js:
     14        (WI.TreeOutline):
     15        (WI.TreeOutline.prototype.insertChild):
     16        Update the SelectionController with the newly inserted index before
     17        attaching the TreeElement. Attaching the TreeElement can cause it to
     18        become selected, which would add the index to the SelectionController,
     19        only to have it immediately incremented by the call to `didInsertItem`.
     20        Additionally, change `insertionIndex` to be the index of the inserted
     21        item instead of the inserted item's previous sibling.
     22
     23        (WI.TreeOutline.prototype._rememberTreeElement):
     24        (WI.TreeOutline.prototype._forgetTreeElement):
     25        (WI.TreeOutline.prototype._indexOfTreeElement.previousElement): Deleted.
     26        Eliminate TreeElement index caching, which could become stale and cause
     27        the wrong index to be calculated. Additionally, instead of walking up the
     28        parent chain to determine the index, start at the root and use existing
     29        method `traverseNextTreeElement`.
     30
    1312018-12-03  Devin Rousso  <drousso@apple.com>
    232
  • trunk/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js

    r238602 r238825  
    204204        let current = this._selectedIndexes.lastIndex;
    205205        while (current >= index) {
    206             this._selectedIndexes.delete(index);
    207             this._selectedIndexes.add(index + 1);
     206            this._selectedIndexes.delete(current);
     207            this._selectedIndexes.add(current + 1);
    208208
    209209            current = this._selectedIndexes.indexLessThan(current);
  • trunk/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js

    r238757 r238825  
    5757        this._cachedNumberOfDescendents = 0;
    5858        this._selectionController = new WI.SelectionController(this);
    59         this._treeElementIndexCache = new Map;
    6059
    6160        this._itemWasSelectedByUser = false;
     
    296295            child.expanded = child.treeOutline._treeElementsExpandedState[child.identifier];
    297296
     297        // Update the SelectionController before attaching the TreeElement.
     298        // Attaching the TreeElement can cause it to become selected, and
     299        // added to the SelectionController.
     300        let insertionIndex = this.treeOutline._indexOfTreeElement(child) || 0;
     301        this.treeOutline._selectionController.didInsertItem(insertionIndex);
     302
    298303        if (this._childrenListNode)
    299304            child._attach();
     
    304309        if (isFirstChild && this.expanded)
    305310            this.expand();
    306 
    307         let insertionIndex = this.treeOutline._indexOfTreeElement(child.previousSibling) || 0;
    308         this.treeOutline._selectionController.didInsertItem(insertionIndex);
    309311    }
    310312
     
    396398    _rememberTreeElement(element)
    397399    {
    398         this._treeElementIndexCache.clear();
    399         this._cachedNumberOfDescendents++;
    400 
    401400        if (!this._knownTreeElements[element.identifier])
    402401            this._knownTreeElements[element.identifier] = [];
     
    409408        // add the element
    410409        elements.push(element);
     410        this._cachedNumberOfDescendents++;
    411411    }
    412412
    413413    _forgetTreeElement(element)
    414414    {
    415         this._treeElementIndexCache.clear();
    416         this._cachedNumberOfDescendents--;
    417 
    418415        if (this.selectedTreeElement === element) {
    419416            element.deselect(true);
    420417            this.selectedTreeElement = null;
    421418        }
    422         if (this._knownTreeElements[element.identifier])
     419        if (this._knownTreeElements[element.identifier]) {
    423420            this._knownTreeElements[element.identifier].remove(element, true);
     421            this._cachedNumberOfDescendents--;
     422        }
    424423    }
    425424
     
    994993    _indexOfTreeElement(treeElement)
    995994    {
    996         function previousElement(element) {
    997             if (element.previousSibling) {
    998                 element = element.previousSibling;
    999                 if (element.children.length)
    1000                     element = element.children.lastValue;
    1001             } else
    1002                 element = element.parent && element.parent.root ? null : element.parent;
    1003             return element;
    1004         }
     995        const skipUnrevealed = false;
     996        const stayWithin = null;
     997        const dontPopulate = true;
    1005998
    1006999        let index = 0;
    1007         let current = treeElement;
     1000        let current = this.children[0];
    10081001        while (current) {
    1009             let closestIndex = this._treeElementIndexCache.get(current);
    1010             if (!isNaN(closestIndex)) {
    1011                 index += closestIndex;
    1012                 break;
    1013             }
    1014 
    1015             current = previousElement(current);
    1016             if (current)
    1017                 index++;
    1018         }
    1019 
    1020         if (!this._treeElementIndexCache.has(treeElement))
    1021             this._treeElementIndexCache.set(treeElement, index);
    1022 
    1023         return index;
     1002            if (treeElement === current)
     1003                return index;
     1004
     1005            current = current.traverseNextTreeElement(skipUnrevealed, stayWithin, dontPopulate);
     1006            ++index;
     1007        }
     1008
     1009        console.assert(false, "Unable to get index for tree element.", treeElement, treeOutline);
     1010        return NaN;
    10241011    }
    10251012
Note: See TracChangeset for help on using the changeset viewer.