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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ehsan.akhgari, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
5.28 KB,
patch
|
dylan
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 2•9 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.
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.
Reporter | ||
Comment 6•9 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•9 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...)
(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.
- 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)
Comment 10•9 years ago
|
||
- add missing css changes
Attachment #8624034 -
Attachment is obsolete: true
Attachment #8624034 -
Flags: review?(dylan)
Attachment #8624037 -
Flags: review?(dylan)
Comment 11•9 years ago
|
||
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+
Comment 12•9 years ago
|
||
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git aa11ef5..43a45ed master -> master
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 13•9 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)
Comment 14•9 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•9 years ago
|
||
This is working very well, thanks!
Updated•5 years ago
|
Component: User Interface: Modal → User Interface
Updated•5 years ago
|
Assignee: glob → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•