Work - implement copy image to clipboard context action

RESOLVED FIXED

Status

defect
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: ally, Assigned: jimm)

Tracking

({feature})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: feature=work)

Attachments

(1 attachment, 1 obsolete attachment)

No description provided.
OS: Mac OS X → Windows 8 Metro
Whiteboard: [metro-mvp] → [metro-mvp][LOE:-]
We are going to have three image context menu items, Copy image (which should copy the  image to the clipboard) "View image in new tab", and "Save image to picture library".
Summary: implement copy image location context action → implement copy image to clipboard context action
Whiteboard: [metro-mvp][LOE:-] → [metro-mvp][LOE:-] feature=work
Blocks: 833165
No longer blocks: metro-contextmenus
Summary: implement copy image to clipboard context action → Work - implement copy image to clipboard context action
Whiteboard: [metro-mvp][LOE:-] feature=work → feature=work
(In reply to Asa Dotzler [:asa] from comment #1)
> We are going to have three image context menu items, Copy image (which
> should copy the  image to the clipboard) "View image in new tab", and "Save
> image to picture library".

We currently have Copy Image Location, which copies the image url to the clipboard. I can implement Copy Image, which would copy the raw bits into the clipboard. If so, should we get of Copy Image Location?
Flags: needinfo?(asa)
I wonder which would be more useful to the user?
(In reply to Jim Mathies [:jimm] from comment #2)
> (In reply to Asa Dotzler [:asa] from comment #1)
> > We are going to have three image context menu items, Copy image (which
> > should copy the  image to the clipboard) "View image in new tab", and "Save
> > image to picture library".
> 
> We currently have Copy Image Location, which copies the image url to the
> clipboard. I can implement Copy Image, which would copy the raw bits into
> the clipboard. If so, should we get of Copy Image Location?

We want copy image to clipboard with the actual bits so you can easily paste into another context. We don't want copy location.
Flags: needinfo?(asa)
Posted patch patch v.1 (obsolete) — Splinter Review
Assignee: nobody → jmathies
Posted patch patch v.1Splinter Review
Attachment #712887 - Attachment is obsolete: true
Comment on attachment 712888 [details] [diff] [review]
patch v.1

I originally tried to get this working using commands, but the code that handles those looks to the root doc for a popup node, which I'm unable to set from our isolated content realm. So I found a way to do it manually down in the 'virtual' content process.
Attachment #712888 - Flags: review?(mbrubeck)
Comment on attachment 712888 [details] [diff] [review]
patch v.1

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

::: browser/metro/base/content/BrowserTouchHandler.js
@@ +22,5 @@
>    onContentContextMenu: function onContentContextMenu(aMessage) {
>      let contextInfo = { name: aMessage.name,
>                          json: aMessage.json,
> +    // Note, target here is the target of the message manager message,
> +    // usually the browser.

Purely for aesthetics, can you move this comment above the "let"?
Attachment #712888 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/mozilla-central/rev/57ebf8b85645
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.