* STEPS TO REPRODUCE: 1. open WebInspector 2. enable the Audit tab 3. select any audit test/group 4. close WebInspector 5. reopen WebInspector 6. select a different audit test/group => both of the `WI.TreeElement` from (3) and (6) are selected
Created attachment 355917 [details] Patch
<rdar://problem/46321795>
Comment on attachment 355917 [details] Patch r-, this doesn't fix the issue. Another reproduction case: 1. open WebInspector 2. enable the Audit tab 3. select "Demo Audit" > "Result Data" > "data-domNodes" 4. close WebInspector 5. reopen WebInspector 6. select "Demo Audit" > "Result Levels" > "level-unsupported" => both "data-domNodes" and "level-unsupported" are selected
This also is the case for the Elements tab: 1. open WebInspector 2. go to the Elements tab 3. select the <body> 4. close WebInspector 5. reopen WebInspector 6. expand the <body> by clicking on the disclosure arrow 6. select any <div> that is a child of <body> => both the <div> and <body> are selected
Created attachment 356347 [details] Patch
This addresses additional defects in SelectionController and TreeOutline, one of which fixes an issue where traversing the tree using the up/down arrow keys would fail to select the previous/next element when the element was on a different level of the tree.
Comment on attachment 356347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356347&action=review r=me > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:400 > this._cachedNumberOfDescendents++; Aside: we should assert that `_cachedNumberOfDescendants` is equal to the sum of the length of every value in `_knownTreeElements`, or even just set it to be that value (in the case that `treeElement` has already been remembered in the past : console.assert(this._cachedNumberOfDescendents === Object.values(this._knownTreeElements).reduce((accumulator, current) => accumulator + current.length, 0)); > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:416 > this._cachedNumberOfDescendents--; Ditto (400) > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:994 > _indexOfTreeElement(treeElement) Aside: the uses of this in `selectionControllerNextSelectableIndex` and `selectionControllerPreviousSelectableIndex` repeat a lot of work, as they both iterate over the tree using `traverseNextTreeElement`. Perhaps consider combining/reworking them (followup)? > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1010 > + return NaN; We should add a "not-reached" assertion: console.assert(false, "Unable to get index for tree element", treeElement, this);
Comment on attachment 356347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356347&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:400 >> this._cachedNumberOfDescendents++; > > Aside: we should assert that `_cachedNumberOfDescendants` is equal to the sum of the length of every value in `_knownTreeElements`, or even just set it to be that value (in the case that `treeElement` has already been remembered in the past : > > console.assert(this._cachedNumberOfDescendents === Object.values(this._knownTreeElements).reduce((accumulator, current) => accumulator + current.length, 0)); I'd like to avoid doing work every time an element is remembered/forgotten. I'll fix the bug where `_cachedNumberOfDescendents` is incremented even if the element had been remembered previously.
Created attachment 356402 [details] Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 356402 [details]: inspector/model/stack-trace.html bug 192332 (authors: drousso@apple.com and joepeck@webkit.org) The commit-queue is continuing to process your patch.
The commit-queue encountered the following flaky tests while processing attachment 356402 [details]: inspector/unit-tests/linked-list.html bug 192333 (authors: joepeck@webkit.org and nvasilyev@apple.com) The commit-queue is continuing to process your patch.
The commit-queue encountered the following flaky tests while processing attachment 356402 [details]: workers/bomb.html bug 192050 (author: fpizlo@apple.com) The commit-queue is continuing to process your patch.
The commit-queue encountered the following flaky tests while processing attachment 356402 [details]: inspector/unit-tests/event-listener.html bug 192338 (authors: bburg@apple.com and drousso@apple.com) imported/w3c/web-platform-tests/html/syntax/parsing/html5lib_webkit01.html bug 192339 (authors: cdumez@apple.com and youennf@gmail.com) The commit-queue is continuing to process your patch.
Comment on attachment 356402 [details] Patch for landing Clearing flags on attachment: 356402 Committed r238825: <https://trac.webkit.org/changeset/238825>
All reviewed patches have been landed. Closing bug.