1012662 - Allow document.execCommand("cut"/"copy") to be used within the context of user generated events

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Hallvord R. M. Steen [:hallvors]]

Per latest draft of the Clipboard Events spec, event listeners for "semi-trusted events" like click should be allowed to modify the clipboard by triggering copy and cut events.

"Triggering" includes document.execCommand('copy|cut') and newer syntax like this Gist demonstrates, once we support that: https://gist.github.com/hallvors/8589631

Comment 1

[Hallvord R. M. Steen [:hallvors]]

Note: the syntax from that Gist will go away - this is purely about document.execCommand() for commands 'copy' and 'cut' now.

It should be possible to simply re-use logic from popup blocking, i.e. events that allow calling window.open() should also allow document.execCommand('copy') and document.execCommand('paste').

Comment 2

[Kohei Yoshino [:kohei]]

Bug 1004260 has been WONTFIXed. Let's track this for Firefox 31, the next ESR.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

It seems to be a new feature to me. 31 will be soon in beta, this could wait for a future release...

Comment 5

[(no longer active)]

My intern who is starting next week is going to work on this.  Vicariously assigning to myself for now.

Comment 6

[(no longer active)]

Reminder: we need to have a test to ensure that execCommand("copy"/"cut") doesn't work on password text fields.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

George, please file and follow the dependent bug where we could remove the mozContentEvent hack in shell.js and rely on this impl. Thanks!

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #7)
> George, please file and follow the dependent bug where we could remove the
> mozContentEvent hack in shell.js and rely on this impl. Thanks!

That however results us calling execCommand() from System app frame in attempt to copy the content of the focused frame. Is this something that we would support or something we should block?

Comment 9

[(Inactive after June) George Duan [:gduan] [:喬智]]

Glad to hear its happening!

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #7)
> George, please file and follow the dependent bug where we could remove the
> mozContentEvent hack in shell.js and rely on this impl. Thanks!
We probably will not remove the hack from shell.js and text_selection_dialog.js unless document.executeCommand can fully support all of copy/paste/selectAll/cut commands.
Will we also impl selectAll/paste in this bug?

Comment 10

[(no longer active)]

(In reply to George Duan [:gduan] [:喬智] from comment #9)
> Will we also impl selectAll/paste in this bug?

No, as the summary of the bug suggests.  :-)

Comment 11

[Hallvord R. M. Steen [:hallvors]]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #6)
> Reminder: we need to have a test to ensure that execCommand("copy"/"cut")
> doesn't work on password text fields.

Hm..interesting, but why? Any JS that has sufficient access to call execCommand() should have even easier access to input.value .. no?

Comment 12

[Hallvord R. M. Steen [:hallvors]]

Also, just to check: when this bug is fixed, will document.queryCommandEnabled('copy') and queryCommandSupported('copy') "just work", or do we need another bug on clarifying what they should do and making sure it goes in? For the feature detection, it is important to release any updates there at the same time as this.

Comment 13

[(no longer active)]

(In reply to Hallvord R. M. Steen [:hallvors] from comment #11)
> (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from
> comment #6)
> > Reminder: we need to have a test to ensure that execCommand("copy"/"cut")
> > doesn't work on password text fields.
> 
> Hm..interesting, but why? Any JS that has sufficient access to call
> execCommand() should have even easier access to input.value .. no?

We do not let the users to copy/cut from a password box (ignoring the fact that we've broken it and now you get to copy the dots!), because that's almost never what they intend to do (copying passwords to the clipboard!) so I think the default behavior of the execCommand API should be the same.  Of course, if the page replaces the data to be copied with their own, then we should copy that.

(In reply to Hallvord R. M. Steen [:hallvors] from comment #12)
> Also, just to check: when this bug is fixed, will
> document.queryCommandEnabled('copy') and queryCommandSupported('copy') "just
> work", or do we need another bug on clarifying what they should do and
> making sure it goes in? For the feature detection, it is important to
> release any updates there at the same time as this.

document.queryCommandSupported currently returns true for all command names that it recognizes, including cut/copy/paste!  Over in bug 1161721 we'll fix that for paste, so calling it for cut and copy will actually return a sane value.

I filed bug 1162952 for queryCommandEnabled, thanks for the reminder!

And yes, we should fix both of those APIs on the same release as the one this is fixed on!

Comment 14

[Nika Layzell [:nika] (ni? for response)]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #13)
> We do not let the users to copy/cut from a password box (ignoring the fact
> that we've broken it and now you get to copy the dots!), because that's
> almost never what they intend to do (copying passwords to the clipboard!) so
> I think the default behavior of the execCommand API should be the same.  Of
> course, if the page replaces the data to be copied with their own, then we
> should copy that.

In that case, I'll have to modify the patch I'm working on - Right now, I don't fire the copy event at all in password fields (which is congruent with the behavior in Chrome). This means that the browser's javascript doesn't even get the chance to put something in the clipboard. 

I could adjust the code such that the copy event fires, but doesn't do anything unless the default is prevented, but that would be different from the other browser's implementations.

The case of password fields is not mentioned in the spec. We should probably file a bug to correct that.

Comment 15

[(no longer active)]

Spec bug: https://github.com/w3c/clipboard-apis/issues/5

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Comment 17

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Updates to clipboard command controllers to match cut/copy action spec

Comment 18

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Tests for new Cut/Copy behaviour

Comment 19

[(no longer active)]

Comment on attachment 8604208 [details] [diff] [review]
Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Review of attachment 8604208 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsContentUtils.h
@@ +1860,4 @@
>    static bool IsRequestFullScreenAllowed();
>  
>    /**
> +   * Returns true if cut or copy execCommands are allowed in the current

Nit: ... if calling execCommand with cut or copy commands ...

::: dom/html/nsHTMLDocument.cpp
@@ +3278,5 @@
> +    if (!nsContentUtils::IsCutCopyAllowed()) {
> +      return false;
> +    }
> +
> +    nsCOMPtr<nsIDocShell> docShell(mDocumentContainer);

Please add a comment explaining why we use nsIDocShell::DoCommand here.  You can point to the logic in nsWindowRoot::GetControllers for choosing where to get the command controllers from in the comment.

Comment 20

[(no longer active)]

Comment on attachment 8604211 [details] [diff] [review]
Part 2: Updates to clipboard command controllers to match cut/copy action spec

Review of attachment 8604211 [details] [diff] [review]:
-----------------------------------------------------------------

Minusing mostly because of the GetBindingParent() call.

::: dom/base/nsCopySupport.cpp
@@ +625,2 @@
>  {
> +  if (aActionTaken) { *aActionTaken = false; }

Nit: please reformat like:

  if (cond) {
    statement;
  }

There are other occurrences of the same style in the patch too.  Please fix all of them!

@@ +718,5 @@
>    uint32_t count = 0;
>    if (doDefault) {
> +    // find the focused node
> +    nsCOMPtr<nsIContent> srcNode = content->GetBindingParent();
> +    if (!srcNode) { srcNode = content; }

Hmm, I don't understand why you're getting the binding parent here.  If you're trying to get out of a native anonymous subtree, this is the wrong way of doing that, since it will also work for XBL bindings, which is probably undesirable here.  In that case, you should probably check IsInNativeAnonymousSubtree() before doing this, and then after it call FindFirstNonChromeOnlyAccessContent().

@@ +748,5 @@
> +      rv = HTMLCopy(sel, doc, aClipboardType, withRubyAnnotation);
> +      if (NS_FAILED(rv)) {
> +        return false;
> +      }
> +    } else { // aType == NS_CUT && !content->IsEditable()

Please remove this comment!

::: dom/base/nsGlobalWindowCommands.cpp
@@ +502,5 @@
>  {
> +  bool isCut = !strcmp(aCommandName, "cmd_cut");
> +
> +  if (!isCut &&
> +      strcmp(aCommandName, "cmd_copy") &&

Nit: since we're using strcmp here already, it seems more consistent to use it for cut as well.

Comment 21

[(no longer active)]

Comment on attachment 8604212 [details] [diff] [review]
Part 3: Tests for new Cut/Copy behaviour

Review of attachment 8604212 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but I have several nits below, and I'd like to see an updated patch addressing them...  Thanks!

::: dom/tests/mochitest/general/test_bug1012662_common.js
@@ +23,5 @@
> +  var range = document.createRange();
> +  range.selectNodeContents(dn);
> +  window.getSelection().removeAllRanges();
> +  window.getSelection().addRange(range);
> +  if (cb) { cb(); }

Nit: please break on braces as mentioned before.

@@ +36,5 @@
> +  });
> +}
> +
> +// Callback functions for attaching to the button
> +function execcut(shouldSucceed) {

Nit: execCut

@@ +45,5 @@
> +    is(shouldSucceed, document.execCommand('cut'), "Keydown caused cut invocation" + testloc());
> +  };
> +  return cb;
> +}
> +function execcopy(shouldSucceed) {

Nit: execCopy

Same elsewhere in the patch...

@@ +83,5 @@
> +    document.querySelector('span').textContent = 'span text';
> +    document.querySelector('input[type=text]').value = 'text text';
> +    document.querySelector('input[type=password]').value = 'password text';
> +    document.querySelector('textarea').value = 'textarea text';
> +    (document.querySelector('div[contentEditable=true]') || {}).textContent = 'contenteditable text';

It seems better to make the check for contenteditable the element explicit.

@@ +190,5 @@
> +
> +      // copy on no selection
> +      document.querySelector('textarea').blur();
> +
> +      waitForClipboard(function(x) { return !/waitForClipboard-known-value/.test(x); },

Hmm, can't we do something better in waitForClipboard to make it unnecessary to pass in this weird argument?

@@ +288,5 @@
> +      return;
> +
> +    default:
> +      cb();
> +      // SimpleTest.finish();

Did you forget to remove this comment?

@@ +326,5 @@
> +        SimpleTest.finish();
> +      }, function(x) { return !/waitForClipboard-known-value/.test(x); }, true);
> +    }, 'overridden');
> +  });
> +  // teststep(0);

This too!

::: testing/mochitest/tests/SimpleTest/SimpleTest.js
@@ +883,4 @@
>    return SimpleTest.__waitForClipboardMonotonicCounter++;
>  });
>  SimpleTest.waitForClipboard = function(aExpectedStringOrValidatorFn, aSetupFn,
> +                                       aSuccessFn, aFailureFn, aFlavor, aTimeout, aShouldFail) {

Nit: you need to document the arguments you're adding to waitForClipboard.

Another nit: please call the last argument aExpectFailure to make it clearer what it does.

Comment 22

[Nika Layzell [:nika] (ni? for response)]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #21)
> @@ +190,5 @@
> > +
> > +      // copy on no selection
> > +      document.querySelector('textarea').blur();
> > +
> > +      waitForClipboard(function(x) { return !/waitForClipboard-known-value/.test(x); },
> 
> Hmm, can't we do something better in waitForClipboard to make it unnecessary
> to pass in this weird argument?

First thing I can think of is adding another option to waitForClipboard where if null is passed in as the first argument, it checks for any change from the original clipboard value - That's what that somewhat odd argument is intended to do. I didn't do that originally, because changing waitForClipboard more than was necessary seemed bad - but I could definitely add some code to SimpleTest such that if aExpectedStringOrValidatorFunction is null, it checks if the clipboard has changed at all.

I'll post the fixed patches without this change, but if you think it's a good idea, I can pretty easily add it in.

Comment 23

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Updates to clipboard command controllers to match cut/copy action spec

Switch to using IsInNativeAnonymousSubtree and FindFirstNonChromeOnlyAccessContent instead of GetBindingParent, style fixes.

Comment 24

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Tests for new Cut/Copy behaviour

Conforming better to style, and improvements to new waitForClipboard semantics.

Comment 25

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Improved comments

Comment 26

[(no longer active)]

(In reply to Michael Layzell [:mystor] from comment #22)
> (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from
> comment #21)
> > @@ +190,5 @@
> > > +
> > > +      // copy on no selection
> > > +      document.querySelector('textarea').blur();
> > > +
> > > +      waitForClipboard(function(x) { return !/waitForClipboard-known-value/.test(x); },
> > 
> > Hmm, can't we do something better in waitForClipboard to make it unnecessary
> > to pass in this weird argument?
> 
> First thing I can think of is adding another option to waitForClipboard
> where if null is passed in as the first argument, it checks for any change
> from the original clipboard value - That's what that somewhat odd argument
> is intended to do. I didn't do that originally, because changing
> waitForClipboard more than was necessary seemed bad - but I could definitely
> add some code to SimpleTest such that if aExpectedStringOrValidatorFunction
> is null, it checks if the clipboard has changed at all.
> 
> I'll post the fixed patches without this change, but if you think it's a
> good idea, I can pretty easily add it in.

We discussed this in person yesterday, and decided to enforce that aExpectedStringOrValidatorFunction should be null if aExpectedFailure is set to true.

Comment 27

[(no longer active)]

Comment on attachment 8605363 [details] [diff] [review]
Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Review of attachment 8605363 [details] [diff] [review]:
-----------------------------------------------------------------

(FWIW, r+ with comments typically means the reviewer trusts that you address the comments, so you don't need to ask for review again.  Usually people will clear the review request or r- when they want to see a new version of the patch.)

Comment 28

[(no longer active)]

Comment on attachment 8605361 [details] [diff] [review]
Part 3: Tests for new Cut/Copy behaviour

Review of attachment 8605361 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks!

Comment 29

[Lawrence Mandel [:lmandel] (use needinfo)]

Release Note Request (optional, but appreciated)
[Why is this notable]: This change will allow programmatic copying and cutting from JS for Web content.
[Suggested wording]: Copy/cut Web content from JavaScript with document.execCommand("cut"/"copy")
[Links (documentation, blog post, etc)]:
Link to standard: This is unfortunately not specified very precisely.
There is a rough spec here: <
https://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#miscellaneous-commands>
and the handling of clipboard events is specified here: <
https://w3c.github.io/clipboard-apis/>.  Sadly, the editing spec is not
actively edited.  We will strive for cross browser interoperability, of
course.

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

Ehsan, I can't tell that this ever landed. What release is it aimed at?

Comment 31

[(no longer active)]

(In reply to Liz Henry (:lizzard) from comment #30)
> Ehsan, I can't tell that this ever landed. What release is it aimed at?

This has not landed yet (it's waiting on bug 1159490, which is waiting for review.)  If it lands in this cycle, it will be in Firefox 41.

Comment 32

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Updated version because of changes to Bug 1162952

New try in progress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b6e1e3067e6

Comment 33

[(no longer active)]

Comment on attachment 8610174 [details] [diff] [review]
Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

Review of attachment 8610174 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/nsHTMLDocument.cpp
@@ +3277,5 @@
> +    // The code past taken by other commands in ExecCommand always uses the window directly,
> +    // rather than deferring to the textbox, which is desireable for most editor commands,
> +    // but not 'cut' and 'copy' (as those should allow copying out of embedded editors).
> +    // This behaviour is invoked if we call DoCommand directly on the docShell.
> +    nsCOMPtr<nsIDocShell> docShell(mDocumentContainer);

Please null check docShell before using it.

Comment 34

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

null check docShell before using it

Comment 35

[Nika Layzell [:nika] (ni? for response)]

Please checkin after Bugs 1162952 and 1159490.

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/265d0878ecd4
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f0306d29dec
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb03cb33817c

Comment 37

[Carsten Book [:Tomcat]]

backed out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=10146961&repo=mozilla-inbound

Comment 38

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/190dd5e07389
https://hg.mozilla.org/integration/mozilla-inbound/rev/d289abc70393
https://hg.mozilla.org/integration/mozilla-inbound/rev/e32074e5713b

Comment 39

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Tests for new Cut/Copy behaviour

Increased the timeout, as sometimes the tests would timeout on the B2G emulator.

Comment 40

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbd4999d7e97
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd8529bd0d4d
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb118b19a902

Comment 41

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbd4999d7e97
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd8529bd0d4d
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb118b19a902

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cbd4999d7e97
https://hg.mozilla.org/mozilla-central/rev/fd8529bd0d4d
https://hg.mozilla.org/mozilla-central/rev/cb118b19a902

Comment 43

[Xidorn Quan [:xidorn] UTC+11]

Some of the users may still be concerned about the fact that websites can easily clear their clipboard without their permission. Although Flash can already do this, users currently can easily block Flash to make that ineffective. But now they will have no way to block this behavior.

For this reason, I suggest we at least add a pref for these commands, so that users who are really concerned can just switch off this pref to disable websites from manipulating their clipboard. Idea?

Comment 44

[(no longer active)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #43)
> Some of the users may still be concerned about the fact that websites can
> easily clear their clipboard without their permission. Although Flash can
> already do this, users currently can easily block Flash to make that
> ineffective. But now they will have no way to block this behavior.
> 
> For this reason, I suggest we at least add a pref for these commands, so
> that users who are really concerned can just switch off this pref to disable
> websites from manipulating their clipboard. Idea?

Feel free to file a bug about that.  As I have stated on the thread about this on dev-platform, I am not convinced that this is more than just a theoretical concern, and will probably accept a patch that adds a hidden pref for that assuming that it correctly handles the queryCommandSupported/queryCommandEnabled APIs as well, but I don't think this is important enough to spend time on it right now.

Comment 45

[Jean-Yves Perrier [:teoli]]

Doc updated:
https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand
and
https://developer.mozilla.org/en-US/Firefox/Releases/41#Interfaces.2FAPIs.2FDOM

Comment 46

[Xidorn Quan [:xidorn] UTC+11]

We probably could also mention in the doc that users who worry about page destroying their clipboard can disable it via setting dom.allow_cut_copy to false. (see bug 1170911)

Comment 47

[(no longer active)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #46)
> We probably could also mention in the doc that users who worry about page
> destroying their clipboard can disable it via setting dom.allow_cut_copy to
> false. (see bug 1170911)

No, I don't think the developer docs are a good place for this.

Comment 48

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Release note added to Firefox 41.0a2

Comment 49

[support]

Could you also allow this to be called in setTimeout.

Chrome supported this which is really convenient:

https://code.google.com/p/chromium/issues/detail?id=424968&q=execCommand%20paste&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Cr%20Status%20Owner%20Summary%20OS%20Modified

Comment 50

[Nika Layzell [:nika] (ni? for response)]

(In reply to support from comment #49)
> Could you also allow this to be called in setTimeout.
> 
> Chrome supported this which is really convenient:
> 
> https://code.google.com/p/chromium/issues/
> detail?id=424968&q=execCommand%20paste&colspec=ID%20Pri%20M%20Stars%20Release
> Block%20Cr%20Status%20Owner%20Summary%20OS%20Modified

Chrome, just like us, supports calling document.execCommand('copy') in response to a user-generated event. This does not include setTimeout, as setTimeout is not a user-generated event. The issue you linked mentions a timeout on the event, meaning that you have to actually perform the copy in a user-generated event callback, and within a certain time horizon. We act the same way.

Comment 51

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Michael Layzell [:mystor] from comment #50)
> (In reply to support from comment #49)
> > Could you also allow this to be called in setTimeout.
> > 
> > Chrome supported this which is really convenient:
> > 
> > https://code.google.com/p/chromium/issues/
> > detail?id=424968&q=execCommand%20paste&colspec=ID%20Pri%20M%20Stars%20Release
> > Block%20Cr%20Status%20Owner%20Summary%20OS%20Modified
> 
> Chrome, just like us, supports calling document.execCommand('copy') in
> response to a user-generated event. This does not include setTimeout, as
> setTimeout is not a user-generated event. The issue you linked mentions a
> timeout on the event, meaning that you have to actually perform the copy in
> a user-generated event callback, and within a certain time horizon. We act
> the same way.

Actually we don't. We never consider an asynchronously call a valid user input callback, but Chrome track that. Hence code like:
> button.onclick = function () {
>   setTimeout(function () {
>     document.execCommand("cut");
>   }, 1000);
> };
works in Chrome, but not in Firefox.

I filed bug 1129227 for Fullscreen API, but actually, popup, fullscreen, and this all share the same logic in our code.

But my question is, is there any real use case which makes asynchronous call to them necessary?

Comment 52

[Nika Layzell [:nika] (ni? for response)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #51)
> Actually we don't. We never consider an asynchronously call a valid user
> input callback, but Chrome track that. Hence code like:
> > button.onclick = function () {
> >   setTimeout(function () {
> >     document.execCommand("cut");
> >   }, 1000);
> > };
> works in Chrome, but not in Firefox.

Oh yes, I see what you mean now. We definitely don't support that in Firefox, didn't realize that chrome supported that.

> I filed bug 1129227 for Fullscreen API, but actually, popup, fullscreen, and
> this all share the same logic in our code.
> 
> But my question is, is there any real use case which makes asynchronous call
> to them necessary?

I am not sure that providing that functionality provides a good user experience, because it means that performing a click can cause "spooky action at a distance", which may be confusing for the user. 

The main use case I can think of, though, would be for a callback for a XHR request sent from the button press (the request would be fetching, for example, the data to copy), does chrome handle that as well?

Nonetheless, I think that this should be a separate bug, and we should move the discussion there.

Comment 53

[support]

It's a must have feature for HTML5 Remote Access software (RDP, VNC, Citrix, VMWare etc). When user copying content from remote computer, the content is passed through websocekt, XHR request and we definitely need this to make it work.

Comment 54

[Hallvord R. M. Steen [:hallvors]]

Let's see if I understand the use case we aren't handling yet: so you have a web page that is actually the local end of a connection to a remote computer. Your web page renders your remote computer's screen and you can interact and issue commands (that are sent with XHR or WebSocket to the remote computer).

You'd like to allow the user to copy some text *from* the remote computer *to* the clipboard on the local one - is that the problem?

Comment 55

[support]

Hi Hallvord. Yes, that's the problem. We have a HTML5 RDP/VNC/SSH client which is used to connect to remote computer. This will allow user to copy local content to remote computer or vice versa.

Citrix, VMWare also have their HTML5 clients too and I believe they have the same problem.

I would like to see execCommand('paste') is supported too. IE does support 'copy' and 'paste' with a permission warning.

I like what IE does for now: There is a permission warning which Chrome and Firefox don't have, after user granted the permission, you can do execCommand('copy or paste') at any time.

Comment 56

[support]

Another thought on the implementation:

Chrome and Firefox all have web store apps. Edge will have too and allow you call Windows API directly.

For the Web Store App, you'll need permission to access local clipboard, but now for normal web pages, user don't need permission (only if it's within a user gesture) you can access local clipboard (copy only). This smells not good for me and that's why I think IE's implementation is better (which can be used for both normal web page and web store app).

Remote desktop clients are used by almost most of the companies and organizations in the world. I really want to see this get supported by Firefox (including the 'paste'). Thanks.

Comment 57

[Xidorn Quan [:xidorn] UTC+11]

I wonder whether we can allow calling execCommand('paste') in a very limited condition, like, when the user press "Ctrl-V" (or "Cmd-V" on OS X).

When user uses that shortcut, he/she should generally expect the content in clipboard is pasted onto the page. I guess we can grant page the permission to read clipboard in the specific event handler if user explicitly does so.

Thoughts?

Comment 58

[Hallvord R. M. Steen [:hallvors]]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #57)
> I wonder whether we can allow calling execCommand('paste') in a very limited
> condition, like, when the user press "Ctrl-V" (or "Cmd-V" on OS X).

When the user presses ctrl-v (or chooses "Edit -> Paste" from a menu) you already get a paste event per spec, and can access several types of data from the clipboard by listening to that event. So your idea already works except you don't need to call execCommand('paste') ;)

(In reply to support from comment #56)
> Chrome and Firefox all have web store apps. Edge will have too and allow you
> call Windows API directly.
> 
> For the Web Store App, you'll need permission to access local clipboard, but
> now for normal web pages, user don't need permission 

I guess the permission for a store app will give you further power than what a web page has - for example permission to use execCommand for copy/cut without user-triggered gestures or use execCommand('paste').

(In reply to support from comment #55)
> We have a HTML5 RDP/VNC/SSH client
> which is used to connect to remote computer. This will allow user to copy
> local content to remote computer or vice versa.

Indeed - this isn't quite smooth at the moment. To sync local to remote, you currently need some UI saying "paste here (Ctrl-V) to send local clipboard contents to remote computer's clipboard". To sync remote to local, you either have to monitor the clipboard on the remote computer and transfer the contents every time they change, to have contents ready for a local copy event - or you need a sort of two-step process where the user first triggers a command to fetch remote clipboard contents and then is told "click here to confirm writing to local clipboard" or something like that.
 
> I like what IE does for now: There is a permission warning which Chrome and
> Firefox don't have, after user granted the permission, you can do
> execCommand('copy or paste') at any time.

Indeed. I think domain whitelisting would be a good enhancement for this feature. Allowing cut/copy in user-triggered threads and reading clipboard contents from native-ui-triggered paste is as far as we can go without a specific "signal of trust" from the user - it's not perfect but it's as far as we get without a whitelist.

Comment 59

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Hallvord R. M. Steen [:hallvors] from comment #58)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #57)
> > I wonder whether we can allow calling execCommand('paste') in a very limited
> > condition, like, when the user press "Ctrl-V" (or "Cmd-V" on OS X).
> 
> When the user presses ctrl-v (or chooses "Edit -> Paste" from a menu) you
> already get a paste event per spec, and can access several types of data
> from the clipboard by listening to that event. So your idea already works
> except you don't need to call execCommand('paste') ;)

That sounds great. If that works, then I don't think we really need to support execCommand('paste').

Comment 60

[(no longer active)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #57)
> I wonder whether we can allow calling execCommand('paste') in a very limited
> condition, like, when the user press "Ctrl-V" (or "Cmd-V" on OS X).

That is pointless, since you can just respond to the "paste" event, which we dispatch when the user presses Ctrl+V.  The use case for execCommand("paste") is for triggering the paste operation if the user is not pressing Ctrl+V.

> When user uses that shortcut, he/she should generally expect the content in
> clipboard is pasted onto the page. I guess we can grant page the permission
> to read clipboard in the specific event handler if user explicitly does so.

We already do.

Comment 61

[(no longer active)]

To answer "support"'s question about pasting, Chrome is experimenting with prompting the user for permission when the page calls this API.  If their experiment proves successful, we may also add support for that in the future.  But I'm not a fan of permission prompts, and I'm skeptical whether that would result in a good user experience.  Beyond that, we're not planning to do something like comment 56, or white-listing domains.  None of those are good solutions for users who just want to go to a web page and paste something.

Comment 62

[Hallvord R. M. Steen [:hallvors]]

(In reply to Michael Layzell [:mystor] from comment #52)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #51)
> > Actually we don't. We never consider an asynchronously call a valid user
> > input callback, but Chrome track that. Hence code like:
> > > button.onclick = function () {
> > >   setTimeout(function () {
> > >     document.execCommand("cut");
> > >   }, 1000);
> > > };
> > works in Chrome, but not in Firefox.
> 
> Oh yes, I see what you mean now. We definitely don't support that in
> Firefox, didn't realize that chrome supported that.

Based on 

data:text/html, <body onclick="setTimeout(function(){window.open('http://example.com');}, 500)">hi</body>

I wonder if setTimeout() tracking already works OK. I vaguely recall Opera (Presto) had to rewrite the popup blocker implementation to support setTimeout() from user-triggered events.

Comment 63

[(no longer active)]

(In reply to Hallvord R. M. Steen [:hallvors] from comment #62)
> (In reply to Michael Layzell [:mystor] from comment #52)
> > (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #51)
> > > Actually we don't. We never consider an asynchronously call a valid user
> > > input callback, but Chrome track that. Hence code like:
> > > > button.onclick = function () {
> > > >   setTimeout(function () {
> > > >     document.execCommand("cut");
> > > >   }, 1000);
> > > > };
> > > works in Chrome, but not in Firefox.
> > 
> > Oh yes, I see what you mean now. We definitely don't support that in
> > Firefox, didn't realize that chrome supported that.
> 
> Based on 
> 
> data:text/html, <body
> onclick="setTimeout(function(){window.open('http://example.com');},
> 500)">hi</body>
> 
> I wonder if setTimeout() tracking already works OK. I vaguely recall Opera
> (Presto) had to rewrite the popup blocker implementation to support
> setTimeout() from user-triggered events.

IIRC the popup blocker doesn't go through the code that we are using here.

Comment 64

[support]

Just my 2 cents for Permission prompt. I think it's not that bad:

1. It prompts when you are using the web page, and you are more sure about your decision at that time. You only need to do it once on this domain so it's not that troublesome.
2. You can accept or refuse this permission and still use the other features, not like you'll have to accept all permissions before you installing it from web store.
3. You don't need to go through the settings to find the permission or white list. Most of normal users will not do that or don't know how.
4. This works on both normal web page and web store app.
5. This follows my rule: User can use the software without tutorials or help.

At the beginning, I pretty liked the permission on Android, but you finally found, you'll have to accept all the permissions if you want to use some apps which make the permission pointless.

Comment 65

[(no longer active)]

Firstly, note that I talk about "white-listing", I specifically mean maintaining a list of websites in Gecko that are allowed to use this feature, not about permissions set by the user.

About the permission based approach, the difficulty is around asking a question that the user can respond to in an intelligent way.  For example, on the first glance, you might suggest asking "Do you want this website to have access to your clipboard?", and that may sound like an OK solution, except that most users will probably have no clue what the "clipboard" is, and why the browser asks about that, and what are the implications of saying yes, and so on, so they will just click the "Whatever" button without understanding what the prompt means.  Because of this reason, we generally prefer to not prompt and ask users about security related decisions.