Bug 192059 – Web Inspector: Elements tab should allow selecting/deleting multiple DOM nodes

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Summary:
Elements tab should allow selecting/deleting multiple DOM nodes.

This will enable deleting multiple nodes, but other operations (hiding, copying, etc) will be disabled if more than one node is selected.

[Radar WebKit Bug Importer]

<rdar://problem/46294827>

[Matt Baker]

Created attachment 355838 [details]
Patch

[Matt Baker]

Created attachment 355839 [details]
Patch

[Devin Rousso]

Comment on attachment 355839 [details]
Patch

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

r=me, with a few followups:
 - deleting multiple DOM nodes will cause a significant number of errors to appear from StyleDetailsPanel.js:77:
 - the styling of the child list bar (the one under the disclosure arrow) needs to be adjusted to not contrast with multiple selections (e.g. selecting a parent and it's children)
 - expanding/collapsing with arrow keys only affects the most recently selected item
 - deleting an element that is hidden will revert the selection to the first child of the root (e.g. select a child element, collapse the parent, hit delete)
 - deleting an element will put the previous/next element (whichever is available) into a permanently selected state until the user manually selects it again
 - ⌘-A selects the details sidebar instead of the DOM tree
 - ER: I should be able to drag to start a selection after the end (or before the beginning)

> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:-197
> -        this._adjustIndexesAfter(index - 1, 1);

Oh lol yeah this would've been an infinite loop 😅

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:67
> +        this._domTreeOutline.allowsEmptySelection = false;

I think this is something we should do for all `WI.TreeOutline`, not just `WI.DOMTreeOutline`.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:370
> -        let indicatesTreeOutlineState = this.treeOutline && (this.treeOutline.dragOverTreeElement === this || this.treeOutline.selectedTreeElement === this || this._animatingHighlight);
> +        let indicatesTreeOutlineState = this.treeOutline && (this.treeOutline.dragOverTreeElement === this || this.selected || this._animatingHighlight);

Are you sure that these are always equivalent?

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:-613
> -            if (!handled && this.treeOutline.ondelete)
> -                handled = this.treeOutline.ondelete(this.selectedTreeElement);

This is used by the breakpoints list in `WI.DebuggerSidebarPanel`.

    for (let treeElement of this.selectedTreeElements) {
        if (treeElement.ondelete && treeElement.ondelete())
            handled = true;
    }

    if (!handled && this.treeOutline.ondelete)
        handled = this.treeOutline.ondelete(this.selectedTreeElement);

[Devin Rousso]

(In reply to Devin Rousso from comment #4)
> r=me, with a few followups:
Another one I just discovered:
 - expanding with right/left arrows will put the element into a permanently selected state until the user manually selects it again

[Matt Baker]

Created attachment 355848 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 355848 [details]:

workers/bomb.html bug 171985 (author: [e-mail])
webgl/1.0.2/conformance/more/functions/uniformMatrixBadArgs.html bug 192072 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 355848 [details]
Patch for landing

Clearing flags on attachment: 355848

Committed r238602: <https://trac.webkit.org/changeset/238602>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.