800977 - Work - implement copy image to clipboard context action

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Comment 1

[Asa Dotzler [:asa]]

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".

Comment 2

[Jim Mathies [:jimm]]

(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?

Comment 3

[Jim Mathies [:jimm]]

I wonder which would be more useful to the user?

Comment 4

[Asa Dotzler [:asa]]

(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.

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 6

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 7

[Jim Mathies [:jimm]]

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.

Comment 8

[Matt Brubeck (: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"?

Comment 9

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57ebf8b85645

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/57ebf8b85645