Use document.execCommand("copy") instead of flash where it is available

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
User Interface: Modal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Ehsan, Assigned: glob)

Tracking

Production

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
We're opening up document.execCommand("copy") in bug 1012662 to all Web pages.  It would be nice if we can dogfood this on Bugzilla.
Blocks: 1149593
(Assignee)

Comment 1

3 years ago
can do, but we unfortunately won't be able to drop flash completely as we have to support other/older browsers.  touching summary to reflect that.
Component: User Interface → User Interface: Modal
Summary: Please don't use Flash to copy text! → Use document.execCommand("copy") instead of flash where it is available
(Reporter)

Comment 2

3 years ago
(In reply to Byron Jones ‹:glob› from comment #1)
> can do, but we unfortunately won't be able to drop flash completely as we
> have to support other/older browsers.  touching summary to reflect that.

Yep. This can be feature detected by wrapping the execCommand call in a try/catch block.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1173347
(Assignee)

Updated

3 years ago
Assignee: nobody → glob
(Assignee)

Comment 4

3 years ago
this looks promising, but it copies the _selected text_ into the clipboard.

as far as i can tell this means we need to put the text into a visible and enabled form field in order for this to work.  setting the field to display:none has the result of the execCommand returning true, but not setting the clipboard.

ehsan:
- is returning false when the clipboard hasn't been updated a bug (ie. should i file it) ?
- is there a way to take content from a js var and put it into the clipboard without any visible changes to the page?
Flags: needinfo?(ehsan)
(Assignee)

Comment 5

3 years ago
(In reply to Byron Jones ‹:glob› from comment #4)
> - is there a way to take content from a js var and put it into the clipboard
> without any visible changes to the page?

note i can work around this by having a zero-sized container for the field, set it to visible, select the text, copy, then hide it again.  this seems like a hack, and i hope there's a cleaner way.
(Assignee)

Updated

3 years ago
No longer blocks: 1149593
(Reporter)

Comment 6

3 years ago
(In reply to Byron Jones ‹:glob› from comment #4)
> this looks promising, but it copies the _selected text_ into the clipboard.

Correct.

> as far as i can tell this means we need to put the text into a visible and
> enabled form field in order for this to work.  setting the field to
> display:none has the result of the execCommand returning true, but not
> setting the clipboard.

Yes.  We try to do the right thing by not copying invisible stuff to the screen.  You don't need to make this display: none though, do you?

> ehsan:
> - is returning false when the clipboard hasn't been updated a bug (ie.
> should i file it) ?

There is no bug, really.  Gecko can't know what you intended for it to copy, so it returns true if the copy operation was successful.  A successful copy operation can obviously mean that nothing was copied to the clipboard, because there was nothing to copy.

> - is there a way to take content from a js var and put it into the clipboard
> without any visible changes to the page?

Not currently.  But something along the lines of comment 5 should come to your rescue.
Flags: needinfo?(ehsan)

Comment 7

3 years ago
(In reply to Byron Jones ‹:glob› from comment #1)
> can do, but we unfortunately won't be able to drop flash completely as we
> have to support other/older browsers.  touching summary to reflect that.

The version info given on https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand was out of date - I've since updated it with that from https://developers.google.com/web/updates/2015/04/cut-and-copy-commands .

I'm pretty sure the set of people that:
* use BMO (vs upstream, seeing as this is a BMO-only UI)
* have flash enabled (and if click to play, have given permissions for BMO)
* have opted into the new UI (yes I imagine we'll make it the default at some point, but by then even fewer people will be on an older browser)
* would actually use the button regularly and/or notice it wasn't there (given this feature wasn't available in the old UI)
* don't use Firefox 41+, Chrome 43+ or Opera 29+
...is pretty small.

As such I personally think the flash support should just be removed when this bug is implemented, but that's just me :-)
(The downfall of flash cannot come soon enough...)
(Assignee)

Comment 8

3 years ago
(In reply to Ed Morley [:emorley] from comment #7)
> The downfall of flash cannot come soon enough...

i 100% agree with that part of your comment, but we'll have to carry flash as a fallback for the time being -- i expect the modal view will be the default before firefox, chrome, and IE have execCommand.copy support in their stable versions.
(Assignee)

Comment 9

3 years ago
Created attachment 8624034 [details] [diff] [review]
1161797_1.patch

- probe for execCommand("copy") and use that when available
- fallback to flash if enabled
- hide the 'copy summary' button otherwise
Attachment #8624034 - Flags: review?(dylan)
(Assignee)

Comment 10

3 years ago
Created attachment 8624037 [details] [diff] [review]
1161797_2.patch

- add missing css changes
Attachment #8624034 - Attachment is obsolete: true
Attachment #8624034 - Flags: review?(dylan)
Attachment #8624037 - Flags: review?(dylan)
Comment on attachment 8624037 [details] [diff] [review]
1161797_2.patch

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

r=dylan
Attachment #8624037 - Flags: review?(dylan) → review+
(Assignee)

Comment 12

3 years ago
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   aa11ef5..43a45ed  master -> master
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 13

3 years ago
¡Hola Byron!

Why do I still get the click-to-play UI on this very bug?

I've set Flash to "Ask to Activate" on about:addons

¡Gracias!
Flags: needinfo?(glob)
(Assignee)

Comment 14

3 years ago
(In reply to alex_mayorga from comment #13)
> Why do I still get the click-to-play UI on this very bug?

this change has been committed but not pushed to production yet.  it should be mid next week.
Flags: needinfo?(glob)
(Reporter)

Comment 15

3 years ago
This is working very well, thanks!
You need to log in before you can comment on or make changes to this bug.