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

RESOLVED FIXED in Firefox 41

Status

()

Core
DOM: Events
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: hallvors, Assigned: mystor)

Tracking

(Depends on: 1 bug, {dev-doc-complete})

unspecified
mozilla41
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox31-, firefox41 fixed, relnote-firefox 41+)

Details

(URL)

Attachments

(3 attachments, 6 obsolete attachments)

10.63 KB, patch
Ehsan
: review+
Details | Diff | Splinter Review
2.63 KB, patch
Details | Diff | Splinter Review
18.15 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
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
(Reporter)

Updated

3 years ago
See Also: → bug 1004260

Updated

3 years ago
OS: Windows 8.1 → All
Hardware: x86_64 → All

Updated

3 years ago
Component: DOM: Security → DOM: Events
(Reporter)

Comment 1

3 years ago
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').
(Reporter)

Updated

3 years ago
See Also: → bug 1013165
Bug 1004260 has been WONTFIXed. Let's track this for Firefox 31, the next ESR.
tracking-firefox31: --- → ?
It seems to be a new feature to me. 31 will be soon in beta, this could wait for a future release...
tracking-firefox31: ? → -

Updated

3 years ago
Blocks: 1030710
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1151429
My intern who is starting next week is going to work on this.  Vicariously assigning to myself for now.
Assignee: nobody → ehsan
Summary: implement click-to-copy and click-to-cut permissions (semi-trusted events) → Allow document.execCommand("cut"/"copy") to be used within the context of user generated events
See Also: → bug 1159490
Assignee: ehsan → michael
Blocks: 1161797
Reminder: we need to have a test to ensure that execCommand("copy"/"cut") doesn't work on password text fields.
Keywords: dev-doc-needed
George, please file and follow the dependent bug where we could remove the mozContentEvent hack in shell.js and rely on this impl. Thanks!
Flags: needinfo?(gduan)
(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?
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?
Flags: needinfo?(gduan)
(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.  :-)
(Reporter)

Comment 11

2 years ago
(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?
(Reporter)

Comment 12

2 years ago
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.
Flags: needinfo?(ehsan)
(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!
Flags: needinfo?(ehsan)
(Assignee)

Comment 14

2 years ago
(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.
Spec bug: https://github.com/w3c/clipboard-apis/issues/5
(Assignee)

Comment 16

2 years ago
Created attachment 8604208 [details] [diff] [review]
Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers
Attachment #8604208 - Flags: review?(ehsan)
(Assignee)

Comment 17

2 years ago
Created attachment 8604211 [details] [diff] [review]
Part 2: Updates to clipboard command controllers to match cut/copy action spec
Attachment #8604211 - Flags: review?(ehsan)
(Assignee)

Comment 18

2 years ago
Created attachment 8604212 [details] [diff] [review]
Part 3: Tests for new Cut/Copy behaviour
Attachment #8604212 - Flags: review?(ehsan)
(Assignee)

Updated

2 years ago
Depends on: 1162952, 1159490
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.
Attachment #8604208 - Flags: review?(ehsan) → review+
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.
Attachment #8604211 - Flags: review?(ehsan) → review-
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.
Attachment #8604212 - Flags: review?(ehsan) → feedback+
(Assignee)

Comment 22

2 years ago
(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.
Flags: needinfo?(ehsan)
(Assignee)

Comment 23

2 years ago
Created attachment 8605307 [details] [diff] [review]
Part 2: Updates to clipboard command controllers to match cut/copy action spec

Switch to using IsInNativeAnonymousSubtree and FindFirstNonChromeOnlyAccessContent instead of GetBindingParent, style fixes.
Attachment #8604211 - Attachment is obsolete: true
Attachment #8605307 - Flags: review?(ehsan)
(Assignee)

Comment 24

2 years ago
Created attachment 8605361 [details] [diff] [review]
Part 3: Tests for new Cut/Copy behaviour

Conforming better to style, and improvements to new waitForClipboard semantics.
Attachment #8604212 - Attachment is obsolete: true
Attachment #8605361 - Flags: review?(ehsan)
(Assignee)

Comment 25

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

Improved comments
Attachment #8604208 - Attachment is obsolete: true
Attachment #8605363 - Flags: review?(ehsan)
(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.
Flags: needinfo?(ehsan)
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.)
Attachment #8605363 - Flags: review?(ehsan) → review+
Attachment #8605307 - Flags: review?(ehsan) → review+
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!
Attachment #8605361 - Flags: review?(ehsan) → review+
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.
relnote-firefox: --- → ?
Ehsan, I can't tell that this ever landed. What release is it aimed at?
Flags: needinfo?(ehsan)
(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.
Flags: needinfo?(ehsan)
(Assignee)

Comment 32

2 years ago
Created attachment 8610174 [details] [diff] [review]
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
Attachment #8605363 - Attachment is obsolete: true
Attachment #8610174 - Flags: review?(ehsan)
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.
Attachment #8610174 - Flags: review?(ehsan) → review+
(Assignee)

Comment 34

2 years ago
Created attachment 8610240 [details] [diff] [review]
Part 1: Allow document.execCommand('copy'/'cut') in user-generated event handlers

null check docShell before using it
Attachment #8610174 - Attachment is obsolete: true
(Assignee)

Comment 35

2 years ago
Please checkin after Bugs 1162952 and 1159490.
Keywords: checkin-needed
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
Keywords: checkin-needed
backed out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=10146961&repo=mozilla-inbound
Flags: needinfo?(michael)

Comment 38

2 years ago
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
(Assignee)

Comment 39

2 years ago
Created attachment 8611192 [details] [diff] [review]
Part 3: Tests for new Cut/Copy behaviour

Increased the timeout, as sometimes the tests would timeout on the B2G emulator.
Attachment #8605361 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8611192 - Attachment description: Tests for new Cut/Copy behaviour → Part 3: Tests for new Cut/Copy behaviour
Flags: needinfo?(michael)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 40

2 years ago
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
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/cbd4999d7e97
https://hg.mozilla.org/mozilla-central/rev/fd8529bd0d4d
https://hg.mozilla.org/mozilla-central/rev/cb118b19a902
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox41: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
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?
Flags: needinfo?(ehsan)
(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.
Flags: needinfo?(ehsan)
Blocks: 1170911
Blocks: 1121463
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
Keywords: dev-doc-needed → dev-doc-complete
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)
(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.
Blocks: 1173313
Release note added to Firefox 41.0a2
relnote-firefox: ? → 41+

Comment 49

2 years ago
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
(Assignee)

Comment 50

2 years ago
(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.
(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?
Flags: needinfo?(support)
(Assignee)

Comment 52

2 years ago
(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

2 years ago
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.
Flags: needinfo?(support)
(Reporter)

Comment 54

2 years ago
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?
Flags: needinfo?(support)

Comment 55

2 years ago
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.
Flags: needinfo?(support)

Comment 56

2 years ago
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.
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?
Flags: needinfo?(support)
Flags: needinfo?(ehsan)
(Reporter)

Comment 58

2 years ago
(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.
(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').
Flags: needinfo?(support)
Flags: needinfo?(ehsan)
(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.
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.
(Reporter)

Comment 62

2 years ago
(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.
(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

2 years ago
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.
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.
(Assignee)

Updated

2 years ago
Blocks: 1223041

Updated

a year ago
Depends on: 1279771
You need to log in before you can comment on or make changes to this bug.