Web Inspector: Elements: allow nodes to be copied and pasted

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

I would expect this to copy the HTML markup (copy) and create a new node (paste) right after the selected node with the copied markup.

This can be achieved right now by right-clicking on a node, selecting "Copy > HTML", right-clicking on the "destination" node, selecting "Add > Next Sibling", and pasting.

[Devin Rousso]

Created attachment 372819 [details]
Patch

[Matt Baker]

(In reply to Devin Rousso from comment #1)
> Created attachment 372819 [details]
> Patch

I never knew I wanted this until testing this patch. Repeatedly hitting paste to keep adding nodes is great.

Do we want to select the pasted node automatically? I was a little surprised after hitting paste and not seeing the selection change.

[Matt Baker]

(In reply to Matt Baker from comment #2)
> (In reply to Devin Rousso from comment #1)
> > Created attachment 372819 [details]
> > Patch
> 
> I never knew I wanted this until testing this patch. Repeatedly hitting
> paste to keep adding nodes is great.
> 
> Do we want to select the pasted node automatically? I was a little surprised
> after hitting paste and not seeing the selection change.

Just checked, and Finder selects the newly pasted items in this case.

[Matt Baker]

Comment on attachment 372819 [details]
Patch

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

r-, because of the copy-paste error in WI._paste.

You can add this in a follow up if you want, but I noticed this doesn't work with comment nodes.

> Source/WebInspectorUI/UserInterface/Base/Main.js:2629
> +    if (selection.isCollapsed && !WI.isEventTargetAnEditableField(event)) {

Decrease indenting with early return:

if (!selection.isCollapsed || WI.isTargetAnEditableField(event))
    return;

> Source/WebInspectorUI/UserInterface/Base/Main.js:2631
> +        if (focusedCopyHandler && typeof focusedCopyHandler.handlePasteEvent === "function") {

Was this copied from WI._copy? The comment above looks wrong too.

> Source/WebInspectorUI/UserInterface/Base/Main.js:2649
> +        return;

Not needed.

[Devin Rousso]

Comment on attachment 372819 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Base/Main.js:2629
>> +    if (selection.isCollapsed && !WI.isEventTargetAnEditableField(event)) {
> 
> Decrease indenting with early return:
> 
> if (!selection.isCollapsed || WI.isTargetAnEditableField(event))
>     return;

This was mainly done to have a consistent style with `WI._copy`, but there's no need for it really.  I'll change it.

>> Source/WebInspectorUI/UserInterface/Base/Main.js:2631
>> +        if (focusedCopyHandler && typeof focusedCopyHandler.handlePasteEvent === "function") {
> 
> Was this copied from WI._copy? The comment above looks wrong too.

Oops. :|

[Devin Rousso]

(In reply to Matt Baker from comment #4)
> You can add this in a follow up if you want, but I noticed this doesn't work with comment nodes.
You mean pasting when a comment node is focused?  Interesting 🤔

This is because `insertAdjacentHTML` only exists on Element nodes, which a Comment is not.  Perhaps we can be smarter with `insertAdjacentHTML` to adjust the `position` based on the type of node (e.g. use the parent or next sibling if the selected node is a comment).

<https://webkit.org/b/199396>

[Devin Rousso]

(In reply to Matt Baker from comment #2)
> Do we want to select the pasted node automatically? I was a little surprised after hitting paste and not seeing the selection change.
Yes, we probably should, but there's a lot of work that would need to be done to properly ensure that we select the node.  Right now, `insertAdjacentHTML` doesn't send any information back to the frontend about the id (or any other data) of the newly added node(s).  As such, we'd basically have to wait for the DOM tree to update, and select the proper node as such.  I think this may be safer/simpler to address as a followup.  <https://webkit.org/b/199397>

[Devin Rousso]

Created attachment 373306 [details]
Patch

[Matt Baker]

Comment on attachment 373306 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 373306 [details]
Patch

Clearing flags on attachment: 373306

Committed r247054: <https://trac.webkit.org/changeset/247054>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52527047>