Closed Bug 1161797 Opened 9 years ago Closed 9 years ago

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

Categories

(bugzilla.mozilla.org :: User Interface, defect)

Production
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ehsan.akhgari, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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
(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: nobody → glob
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)
(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.
No longer blocks: 1149593
(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)
(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...)
(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.
Attached patch 1161797_1.patch (obsolete) — Splinter Review
- 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)
Attached patch 1161797_2.patchSplinter Review
- 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+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   aa11ef5..43a45ed  master -> master
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
¡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)
(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)
This is working very well, thanks!
Component: User Interface: Modal → User Interface
Assignee: glob → nobody
You need to log in before you can comment on or make changes to this bug.