[webkit-reviews] review denied: [Bug 192917] Web Inspector: REGRESSION: deleting an audit puts selection in a selected but invisible state : [Attachment 358026] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 3 11:34:40 PST 2019


Devin Rousso <drousso at apple.com> has denied Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 192917: Web Inspector: REGRESSION: deleting an audit puts selection in a
selected but invisible state
https://bugs.webkit.org/show_bug.cgi?id=192917

Attachment 358026: Patch

https://bugs.webkit.org/attachment.cgi?id=358026&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 358026
  --> https://bugs.webkit.org/attachment.cgi?id=358026
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358026&action=review

r-, as I think there's a simpler (and safer) approach to `removeChildren`,
namely just entirely wiping out the `_selectionController`'s list of
`_selectedIndexes`.  Since we're removing all children from the
`WI.TreeOutline`, there shouldn't be anything left to select, meaning that all
we really need to do is call `this._selectionController.deselectAll`.

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1103
> +	   // Include the index of the subtree's root, unless it's the
TreeOutline root.
> +	   if (!treeElement.root)
> +	       startIndex--;

Also, why is this needed?  None of the other index-related functions care about
the `root`?  This should either be removed or have a better comment.

Also, AFACIT, the root is only set on `WI.TreeOutline`, so I don't see how this
would ever not get called?


More information about the webkit-reviews mailing list