Bug 192059

Summary: Web Inspector: Elements tab should allow selecting/deleting multiple DOM nodes
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P3 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 191483    
Bug Blocks: 192121, 192112, 192114, 192116, 192118, 192119, 192120, 192353, 192354    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Matt Baker 2018-11-27 17:07:44 PST
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.
Comment 1 Radar WebKit Bug Importer 2018-11-27 17:08:22 PST
<rdar://problem/46294827>
Comment 2 Matt Baker 2018-11-27 18:55:36 PST
Created attachment 355838 [details]
Patch
Comment 3 Matt Baker 2018-11-27 19:04:53 PST
Created attachment 355839 [details]
Patch
Comment 4 Devin Rousso 2018-11-27 19:34:04 PST
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);
Comment 5 Devin Rousso 2018-11-27 19:36:19 PST
(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
Comment 6 Matt Baker 2018-11-27 21:59:34 PST
Created attachment 355848 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2018-11-27 22:48:59 PST
The commit-queue encountered the following flaky tests while processing attachment 355848 [details]:

workers/bomb.html bug 171985 (author: fpizlo@apple.com)
webgl/1.0.2/conformance/more/functions/uniformMatrixBadArgs.html bug 192072 (author: roger_fong@apple.com)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2018-11-27 22:49:42 PST
Comment on attachment 355848 [details]
Patch for landing

Clearing flags on attachment: 355848

Committed r238602: <https://trac.webkit.org/changeset/238602>
Comment 9 WebKit Commit Bot 2018-11-27 22:49:43 PST
All reviewed patches have been landed.  Closing bug.