Bug 156529 – document.execCommand("copy") only triggers if there is a selection

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Lucas Garron]

Safari 9.1.1 
OSX 10.11.4

1. Visit https://permission.site
2. Click on "Copy"

Expected:
The text "This text was copied from the permission.site clipboard example." is placed on the clipboard.

Actual:
No change in the clipboard.

However, if you select (any) text on the page before step 2, then step 2 will result in the expected behaviour.

It appears that document.execCommand("copy") does not fire a copy event if there is no selection on the current page.
Thus, the following idiom does not work as expected if there is no selection at the time you copy:

      document.addEventListener("copy", function(e){
            e.clipboardData.setData("text/plain", "test");
			e.preventDefault();
      });

      var copyButton = document.createElement("button");
      document.body.appendChild(copyButton);

      copyButton.addEventListener("click", function() {
        document.execCommand("copy");
      });

(Firefox, Chrome, Opera, and IE/Edge all trigger the copy event even when there is no selection.)

[Lucas Garron]

Gentle ping.
Is this working as intended?

If not, it would be nice if Safari could match the behaviour of other browsers for minimal dev surprise/workaround needs.

[Radar WebKit Bug Importer]

<rdar://problem/27792460>

[Ryosuke Niwa]

This stems from the fact WebKit uses selection to determine the copy/paste target whereas other browsers use the focused element to the same.

We should probably tweak Editor::dispatchCPPEvent so that it falls back to the focused element or the main document when there is no selection.

[Lucas Garron]

Gentle ping. :-)

Since this shipped with Safari 10, I had to add the following workaround to my clipboard shim library [1]:


    // Workaround for Safari: https://bugs.webkit.org/show_bug.cgi?id=156529
    function bogusSelect() {
      var sel = document.getSelection();
      // If "nothing" is selected...
      if (!document.queryCommandEnabled("copy") && sel.isCollapsed) {
        // ... temporarily select the entire body.
        //
        // We select the entire body because:
        // - it's guaranteed to exist,
        // - it works (unlike, say, document.head, or phantom element that is
        //   not inserted into the DOM),
        // - it doesn't seem to flicker (due to the synchronous copy event), and
        // - it avoids modifying the DOM (can trigger mutation observers).
        //
        // Because we can't do proper feature detection (we already checked
        // document.queryCommandEnabled("copy") , which actually gives a false
        // negative for Blink when nothing is selected) and UA sniffing is not
        // reliable (a lot of UA strings contain "Safari"), this will also
        // happen for some browsers other than Safari. :-()
        var range = document.createRange();
        range.selectNodeContents(document.body);
        sel.addRange(range);
        _bogusSelection = true;
      }
    };

This feels very fragile. :-(




[1] https://github.com/lgarron/clipboard.js

[Lucas Garron]

Actually, it seems that Safari unconditionally returns false for `document.queryCommandEnabled("copy")`, which should probably be considered a separate bug.

[Daniel Bates]

This is because:

[[
bool Editor::canCopy() const
{
    if (imageElementFromImageDocument(document()))
        return true;
    const VisibleSelection& selection = m_frame.selection().selection();
    return selection.isRange() && !selection.isInPasswordField();
}
]]
<https://trac.webkit.org/browser/trunk/Source/WebCore/editing/Editor.cpp?rev=249040#L481>

[Daniel Bates]

Something I need to understand,

[Daniel Bates]

(In reply to Daniel Bates from comment #7)
> Something I need to understand,

Never mind this.

[Daniel Bates]

(In reply to Daniel Bates from comment #6)
> This is because:
> 
> [[
> bool Editor::canCopy() const
> {
>     if (imageElementFromImageDocument(document()))
>         return true;
>     const VisibleSelection& selection = m_frame.selection().selection();
>     return selection.isRange() && !selection.isInPasswordField();
> }
> ]]
> <https://trac.webkit.org/browser/trunk/Source/WebCore/editing/Editor.
> cpp?rev=249040#L481>

I think this is too-low... It's a common function also used in making decisions of menu. Need to look one caller up next... enabledCopy()

[Daniel Bates]

(In reply to Daniel Bates from comment #9)
> (In reply to Daniel Bates from comment #6)
> > This is because:
> > 
> > [[
> > bool Editor::canCopy() const
> > {
> >     if (imageElementFromImageDocument(document()))
> >         return true;
> >     const VisibleSelection& selection = m_frame.selection().selection();
> >     return selection.isRange() && !selection.isInPasswordField();
> > }
> > ]]
> > <https://trac.webkit.org/browser/trunk/Source/WebCore/editing/Editor.
> > cpp?rev=249040#L481>
> 
> I think this is too-low... It's a common function also used in making
> decisions of menu. Need to look one caller up next... enabledCopy()

Just to clarify, I'm talking about finding where we need to patch up the code to fix this bug. Fact is we end up in canCopy() today and this effdcts enabledCopy()'s decision on whether to allow the copy.

[Ryosuke Niwa]

So the way execCommand works is to via executeCopy in EditorCommand.cpp, which calls Editor::copy() but you're right that enabledCopy in EditorCommand.cpp is what decides whether copy is enabled or not.

I think what you want to do is to adjust Editor::findEventTargetFromSelection() which currently relies on the selection to find the target. Add some argument to canDHTMLCut() and canDHTMLCopy() or add an argument to findEventTargetFromSelection so that it can fallback to the focused element or body as needed.

[Tim Nguyen (:ntim)]

Pull request: https://github.com/WebKit/WebKit/pull/38127

[EWS]

Committed 288559@main (03472c5b0900): <https://commits.webkit.org/288559@main>

Reviewed commits have been landed. Closing PR #38127 and removing active labels.