Bug 192059 - Web Inspector: Elements tab should allow selecting/deleting multiple DOM nodes
Summary: Web Inspector: Elements tab should allow selecting/deleting multiple DOM nodes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P3 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on: 191483
Blocks: 192121 192112 192114 192116 192118 192119 192120 192353 192354
  Show dependency treegraph
 
Reported: 2018-11-27 17:07 PST by Matt Baker
Modified: 2018-12-03 22:13 PST (History)
4 users (show)

See Also:


Attachments
Patch (10.09 KB, patch)
2018-11-27 18:55 PST, Matt Baker
no flags Details | Formatted Diff | Diff
Patch (10.09 KB, patch)
2018-11-27 19:04 PST, Matt Baker
no flags Details | Formatted Diff | Diff
Patch for landing (11.48 KB, patch)
2018-11-27 21:59 PST, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.