Bug 156529 - document.execCommand("copy") only triggers if there is a selection.
Summary: document.execCommand("copy") only triggers if there is a selection.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: Safari Technology Preview
Hardware: Mac OS X 10.11
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-12 18:56 PDT by Lucas Garron
Modified: 2019-09-10 17:06 PDT (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Garron 2016-04-12 18:56:09 PDT
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.)
Comment 1 Lucas Garron 2016-08-05 12:57:52 PDT
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.
Comment 2 Radar WebKit Bug Importer 2016-08-10 13:48:33 PDT
<rdar://problem/27792460>
Comment 3 Ryosuke Niwa 2016-08-11 00:09:25 PDT
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.
Comment 4 Lucas Garron 2016-09-23 14:15:45 PDT
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
Comment 5 Lucas Garron 2016-09-23 14:32:06 PDT
Actually, it seems that Safari unconditionally returns false for `document.queryCommandEnabled("copy")`, which should probably be considered a separate bug.
Comment 6 Daniel Bates 2019-09-10 11:02:35 PDT
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>
Comment 7 Daniel Bates 2019-09-10 11:10:52 PDT
Something I need to understand,
Comment 8 Daniel Bates 2019-09-10 11:13:48 PDT
(In reply to Daniel Bates from comment #7)
> Something I need to understand,

Never mind this.
Comment 9 Daniel Bates 2019-09-10 15:44:10 PDT
(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()
Comment 10 Daniel Bates 2019-09-10 15:45:27 PDT
(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.
Comment 11 Ryosuke Niwa 2019-09-10 16:52:08 PDT
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.