Closed Bug 1356543 Opened 3 years ago Closed 2 years ago

Copy image to clipboard (WebExtensions)

Categories

(WebExtensions :: Untriaged, enhancement, P2)

52 Branch
enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed
Blocking Flags:
webextensions ?

People

(Reporter: pag77, Assigned: robwu)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [outreach][moz china][triaged][awe:easyscreenshot@mozillaonline.com])

Attachments

(1 file, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170323105023

Steps to reproduce:

Please add in the WebExtensions API the ability to copy images to the clipboard
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
already possible without a separate API...andy will try to reproduce to give example
Flags: needinfo?(amckay)
I will be grateful for an example!
Flags: needinfo?(amckay)
Whiteboard: [triaged]
> already possible without a separate API...andy will try to reproduce to give example

please send me example for copy images to the clipboard
It looks like there's no way to copy the image..
Please add in the WebExtensions API the ability to copy images to the clipboard
I modified https://github.com/mdn/webextensions-examples/tree/master/context-menu-copy-link-with-types, thinking maybe I could get that to work with images. But I failed. Here's what I tried.

Key point: according to https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer:

> This object is available from the dataTransfer property of all drag events.

I was hoping it might be there for copy events, too, but it's not.

There are actually very few changes between this code and the github example. Test image: http://strabo.com/gallery/albums/wallpaper/foo_wallpaper.sized.jpg

clipboard-helper.js:
--------------------

  // This function must be called in a visible page, such as a browserAction popup
  // or a content script. Calling it in a background page has no effect!
  function copyToClipboard(text, html) {
      function oncopy(event) {
          document.removeEventListener("copy", oncopy, true);
          // Hide the event from the page to prevent tampering.
          event.stopImmediatePropagation();

          // Overwrite the clipboard content.
          event.preventDefault();
          let dt = event.clipboardData.dataTransfer;
          console.log("dt is " + dt); // THIS DISPLAYS "dt is undefined"
          event.clipboardData.mozSetDataAt("image/jpeg", dt.dataTransfer, 0);
    }
    document.addEventListener("copy", oncopy, true);

    // Requires the clipboardWrite permission, or a user gesture:
    document.execCommand("copy");
  }


background.js:
--------------

browser.contextMenus.create({
      id: "copy-link-to-clipboard",
      title: "Copy link to clipboard",
      contexts: ["all"],
  });
  browser.contextMenus.onClicked.addListener(function(info, tab) {
      if (info.menuItemId === "copy-link-to-clipboard") {
          if (typeof info.mediaType === "undefined" || info.mediaType !== "image")
              return;

          console.log("media type is " + info.mediaType); // Correctly outputs "media type is image"

          // The example will show how data can be copied, but since background
          // pages cannot directly write to the clipboard, we will run a content
          // script that copies the actual content.

          // clipboard-helper.js defines function copyToClipboard.
          const code = "copyToClipboard()";

          browser.tabs.executeScript({
              code: "typeof copyToClipboard === 'function';",
          }).then(function(results) {
              // The content script's last expression will be true if the function
              // has been defined. If this is not the case, then we need to run
              // clipboard-helper.js to define function copyToClipboard.
              if (!results || results[0] !== true) {
                  return browser.tabs.executeScript(tab.id, {
                      file: "clipboard-helper.js",
                  });
              }
          }).then(function() {
              return browser.tabs.executeScript(tab.id, {
                  code,
              });
          }).catch(function(error) {
              // This could happen if the extension is not allowed to run code in
              // the page, for example if the tab is a privileged page.
              console.error("Failed to copy: " + error);
          });
      }
  });
I previously tried the same and the same failed.
I also tried the example from stackoverflow:
https://stackoverflow.com/questions/33175909/copy-image-to-clipboard

=============
<script>
        //Cross-browser function to select content
        function SelectText(element) {
            var doc = document;
            if (doc.body.createTextRange) {
                var range = document.body.createTextRange();
                range.moveToElementText(element);
                range.select();
            } else if (window.getSelection) {
                var selection = window.getSelection();
                var range = document.createRange();
                range.selectNodeContents(element);
                selection.removeAllRanges();
                selection.addRange(range);
            }
        }
        $(".copyable").click(function (e) {
            //Make the container Div contenteditable
            $(this).attr("contenteditable", true);
            //Select the image
            SelectText($(this).get(0));
            //Execute copy Command
            //Note: This will ONLY work directly inside a click listenner
            document.execCommand('copy');
            //Unselect the content
            window.getSelection().removeAllRanges();
            //Make the container Div uneditable again
            $(this).removeAttr("contenteditable");
            //Success!!
            alert("image copied!");
        });
</script>
=============

This works, but... this only works for Microsoft Windows Word or other word processors.
Programs for working with graphics (paint, photoshop, etc) do not understand what is on the clipboard: (
Whiteboard: [triaged] → [triaged][outreach]
Whiteboard: [triaged][outreach] → [outreach, moz china]
Blocks: 1311472
Blocks: 1344410
Severity: normal → enhancement
webextensions: --- → ?
Priority: -- → P2
Whiteboard: [outreach, moz china] → [outreach][moz china][triaged]
Hey Boris,

We need to decide how to support copying images to clipboard from web extensions, so can you help us estimate how much work would it be to support "image/png" using the standard web clipboard api https://w3c.github.io/clipboard-apis/#writing-to-clipboard

Is it mostly a question of permission, and wiring things up to allow web extensions callers (in which case I could probably take the bug), or is it more involved, and actually needs to be implemented?
Flags: needinfo?(bzbarsky)
Good question.  The relevant API is DataTransfer's setData, which looks like this:

  void setData(DOMString format, DOMString data);

and is defined at https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-setdata -- note that it only deals with things whose "kind" (in the https://html.spec.whatwg.org/multipage/dnd.html#the-drag-data-item-kind sense) is "Plain Unicode string".  So this API is not really very usable for the use case we're talking about here.

Now we do have some internal Mozilla APIs on DataTransfer that might be a bit more flexible.  See mozSetDataAt.  So using that you should be able to get your bits into the DataTransfer object, at least.

Once you do that, I _think_ nsCopySupport::FireClipboardEvent will do the "put it on the OS clipboard" thing, but only if the copy event's preventDefault() was called.
Flags: needinfo?(bzbarsky)
mozSetDataAt is exactly the same as setData, except it allows the transfer data index to be set.

I'm thinking of the following implementation:
We will allow web pages (not just extensions) to copy an image if it is the only node in a selection, as shown below.
When this case is detected, the logic behind the "Copy Image" context menu item is executed (nsCopySupport::ImageCopy).

function copyImage(img) {
  var range = document.createRange();
  range.selectNode(img);

  var selection = window.getSelection();
  selection.removeAllRanges();
  selection.addRange(range);

  document.execCommand('Copy');
}

Full example (note that the following will only work with an extension and the clipboardWrite permission, because execCommand is executed asynchronously in the img.onload event handler):

img = new Image(); // An <img> element
img.src = 'https://robwu.nl/favicon.ico';
img.onload = function() {
  document.body.appendChild(img);
  copyImage(img);
  img.remove();
};


I am specifically choosing to require a valid <img> element, because allowing arbitrary data to be assigned to the image/* mimeType has been met with concerns before, in https://lists.w3.org/Archives/Public/public-webapps/2015AprJun/0819.html
> mozSetDataAt is exactly the same as setData, except it allows the transfer data index to be set.

Except it accepts any JS value as the data, not just strings.  Though I haven't sorted through what ends up happening if you set something like an <img> element as the data.
<button onclick="copyImage2()">copyImage</button>
<img src="https://robwu.nl/favicon.ico" onclick="copyImage(this);"/>
<script language="javascript" type="text/javascript">
function copyImage(img) {
  var range = document.createRange();
  range.selectNode(img);

  var selection = window.getSelection();
  selection.removeAllRanges();
  selection.addRange(range);

  document.execCommand('Copy');
}

function copyImage2() {
	img = new Image(); // An <img> element
	img.src = 'https://robwu.nl/favicon.ico';
	img.onload = function() {
	  document.body.appendChild(img);
	  copyImage(img);
	  img.remove();
	};
}
</script>
==========
Test 1: Click to button "copyImage"
Error: Warning: document.execCommand('cut'/'copy') was denied because it was not called from inside a short running user-generated event handler.
==========
Test 2: Click to Image
Clipboard is empty
(In reply to Oleksandr from comment #11)

The snippet in comment 9 is not working, it was an API proposal.

:zombie brought up that document.execCommand('copy') should not exhibit the new behavior that I suggested, because doing so might affect applications that consume the output of execCommand (who expect the current format).


I have just started writing with a patch. Manual testing shows that it works, so I'll now improve the code and add unit tests.
In my prototype, I detect a couple of known image formats (png, jpeg/jpg, gif), and store the data in an imgIContainer instead of a string.
The prototype expects the input to be a binary string, i.e. the API is:
event.clipboardData.setData('image/png', 'content of png here');

I am now updating it to (also or only?) recognize Blobs, so that the expected API is:
event.clipboardData.mozSetDataAt('image/png', new Blob(['content of png here']), 0);
This only works on privileged pages at the moment, but I can extend the checks so that it also becomes available for WebExtensions with the clipboardWrite permission.
A concern with building upon mozSetDataAt is that it seems that there is an intent to remove the API (in bug 1335859).
Please keep me informed and let me know when Nightly will be able (If it can) to copy images to the clipboard
Thank you!
Assignee: nobody → rob
:baku, I selected you as reviewer because you also reviewed clipboard+image-related patches for bug 906420.

To minimize backwards-incompatible changes, the image will only be converted in a special way when the MIME type is set to "application/x-moz-nativeimage", see dom/events/test/test_bug1356543.html in the patch for an example.

The feature requires event.clipboardData.mozSetDataAt instead of event.clipboardData.setData, because the latter only accepts a string, whereas I want to save Blobs.
If bug 1345192 ever gets implemented (i.e. mozSetDataAt is removed), then the copy-to-image needs to be implemented in another way, e.g. by parsing binary strings.

:mystor, How likely is it for mozSetDataAt to be removed (bug 1345192)?
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(michael)
See Also: → 1345192
(In reply to Rob Wu [:robwu] from comment #15)
> :mystor, How likely is it for mozSetDataAt to be removed (bug 1345192)?

I don't think it's going to be removed super soon (although the telemetry pings suggest it doesn't have very high usage at all on websites - I don't have enough time to actually verify that the numbers are low enough and go through the process of removing it).

That being said, I think that encouraging webextensions to use our moz-specific APIs which we are planning to remove in order to enable them to copy images to the clipboard is not a good idea. I don't think we should be adding any new abilities to the moz* APIs which is not present in the non-moz APIs. 

Please find another way to enable users to copy images to the clipboard without encouraging the use of an old vendor-specific API which doesn't map well to the future of the web and currently has significant maintenance burden (which is why we want to remove it).
Flags: needinfo?(michael)
Comment on attachment 8882856 [details]
Bug 1356543 - Copy image to clipboard via special MIME type

https://reviewboard.mozilla.org/r/153896/#review159510

::: dom/events/DataTransfer.cpp:42
(Diff revision 2)
>  #include "mozilla/dom/FileList.h"
>  #include "mozilla/dom/BindingUtils.h"
>  #include "mozilla/dom/OSFileSystem.h"
>  #include "mozilla/dom/Promise.h"
>  #include "nsNetUtil.h"
> +#include "imgTools.h"

alphabetic order.

::: dom/events/DataTransfer.cpp:1275
(Diff revision 2)
> +    NS_ENSURE_TRUE(domBlob, false);
> +    blobImpl = static_cast<Blob*>(domBlob.get())->Impl();
> +  }
> +
> +  nsCOMPtr<nsIInputStream> imageStream;
> +  ErrorResult error;

IgnoredErrorResult

::: dom/events/DataTransfer.cpp:1278
(Diff revision 2)
> +
> +  nsCOMPtr<nsIInputStream> imageStream;
> +  ErrorResult error;
> +  blobImpl->GetInternalStream(getter_AddRefs(imageStream), error);
> +  if (NS_WARN_IF(error.Failed())) {
> +    error.SuppressException();

if you use IgnoredErrorResult you can remove this line.
Attachment #8882856 - Flags: review?(amarchesini) → review+
(In reply to Michael Layzell [:mystor] from comment #17)
> (In reply to Rob Wu [:robwu] from comment #15)
> > :mystor, How likely is it for mozSetDataAt to be removed (bug 1345192)?
> 
> I don't think it's going to be removed super soon (although the telemetry
> pings suggest it doesn't have very high usage at all on websites - I don't
> have enough time to actually verify that the numbers are low enough and go
> through the process of removing it).
> 
> That being said, I think that encouraging webextensions to use our
> moz-specific APIs which we are planning to remove in order to enable them to
> copy images to the clipboard is not a good idea. I don't think we should be
> adding any new abilities to the moz* APIs which is not present in the
> non-moz APIs. 
> 
> Please find another way to enable users to copy images to the clipboard
> without encouraging the use of an old vendor-specific API which doesn't map
> well to the future of the web and currently has significant maintenance
> burden (which is why we want to remove it).

I added another patch that supports copying images via the dataTransfer.setData API.
I will make sure that the to-be-created MDN documentation will feature a note that mozSetData might be removed in the future. Developers who really need the characteristics of mozSetData (with Blob/File) should then occasionally check whether the method is still supported.
All other devs (those who want to copy small images, or those who don't care about future compat) can use setData with string arguments.

Note that I have intentionally chosen a very vendor-specific-looking MIME type (application/x-moz-nativeimage), to not pose a maintenance burden in case a standard method to copy images arises. I don't expect such a spec to materialize any time soon, hence I have created an API that is as simple as possible to solve the immediate need of add-ons. Although I mainly had add-ons in mind, this API is also web-exposed (unlike WebExtensions, web pages have to wait for a user-triggered copy/cut event though).

I have filed a request for a specification at https://github.com/w3c/clipboard-apis/issues/44.
:mystor
Are you fine with landing the patches as-is (after they're approved), or do you want to see a fundamental change in the implementation?
The commit message at https://reviewboard.mozilla.org/r/154476/diff/1#index_header sums up the current state of the new functionality, and the previous comment (and my Github issue) provides more context if needed.
Flags: needinfo?(michael)
(In reply to Rob Wu [:robwu] from comment #21)
> Note that I have intentionally chosen a very vendor-specific-looking MIME
> type (application/x-moz-nativeimage), to not pose a maintenance burden in
> case a standard method to copy images arises. I don't expect such a spec to
> materialize any time soon, hence I have created an API that is as simple as
> possible to solve the immediate need of add-ons. Although I mainly had
> add-ons in mind, this API is also web-exposed (unlike WebExtensions, web
> pages have to wait for a user-triggered copy/cut event though).

I am even more strongly opposed to this if you're letting it be web-exposed. We have an explicit policy against shipping moz-prefixed APIs, which this is just another form for: https://wiki.mozilla.org/WebAPI/ExposureGuidelines. 

We shouldn't be trying to provide mozilla-specific APIs which we intend to unship to webextensions, or to web content. Instead we should be working with the standards body to figure out what a good solution looks like. For example, we could work with other browsers to provide a DataTransferItemList::AddImage API which adds the image from a Blob.

If we do decide that we need to support an an extension to the web standard which is used _only for web extensions_, then we should talk about what sort of API we can provide on DataTransfer explicitly for this internal purpose, and make sure that we design it with no dependency on the legacy internal format of DataTransfer. Finally we should definitely not expose this API to web content.
(In reply to Rob Wu [:robwu] from comment #22)
> :mystor
> Are you fine with landing the patches as-is (after they're approved), or do
> you want to see a fundamental change in the implementation?
> The commit message at
> https://reviewboard.mozilla.org/r/154476/diff/1#index_header sums up the
> current state of the new functionality, and the previous comment (and my
> Github issue) provides more context if needed.

I am not OK with these patches landing, and I think that exposing mozilla-specific APIs to the web or encouraging webextensions to use mozilla-specific APIs which we want to remove is a bad idea.
Flags: needinfo?(michael)
Fair p(In reply to Michael Layzell [:mystor] from comment #23)
> We shouldn't be trying to provide mozilla-specific APIs which we intend to
> unship to webextensions, or to web content. Instead we should be working
> with the standards body to figure out what a good solution looks like. For
> example, we could work with other browsers to provide a
> DataTransferItemList::AddImage API which adds the image from a Blob.

I started a discussion at https://github.com/w3c/clipboard-apis/issues/44.

> If we do decide that we need to support an an extension to the web standard
> which is used _only for web extensions_, then we should talk about what sort
> of API we can provide on DataTransfer explicitly for this internal purpose,
> and make sure that we design it with no dependency on the legacy internal
> format of DataTransfer. Finally we should definitely not expose this API to
> web content.

The API takes a Blob/File/string and makes it available on the clipboard for use by image editing applications. There are no other guarantees, or dependencies on a specific internal representation.

If I land this patch at all (i.e. spec discussion stalls with no solution in sight for Firefox 57 (the milestone where legacy add-ons stop working)), then I will move the functionality behind the clipboardWrite permission of WebExtensions, and explicitly document that the API is unstable, including the condition when the API should not be used.
The alternative is to provide a nice WebExtension API (e.g. browser.clipboard) for manipulating the clipboard, but I believe that introducing extension-specific APIs that duplicate existing functionality on the web platform should be avoided.
It's a lot easier for us to deprecate and change APIs that occur in WebExtensions than it is Web APIs. For example we have a copy of every WebExtension and can individually contact all the authors and let them know of the changes. 

That being said, I also agree with :mystor that we shouldn't be exposing Mozilla internal APIs to the web in general.

I do rate being able to copy images to the clipboard as important for WebExtensions for Firefox 57. I see two main paths here:
* make clipboard API that hides the mozilla internals, as the DOM APIs improve eventually we'll be able to change the underlying API code and then remove the API (which I think is what Chrome did)
* we recommend people use the API Rob mentioned and then the DOM APIs improve and we move everyone over to that.

Either way it feels like what we are building now is duct taping around a problem for Firefox 57 and we'll need to change things in the future. 

Stupid question, in comment 25 you mentioned that you could put this behind the clipboardWrite permission, which limits it WebExtensions only. Would we also need to do a similar thing behind a clipboardRead permission to read the image?
(In reply to Andy McKay [:andym] from comment #26)
> Stupid question, in comment 25 you mentioned that you could put this behind
> the clipboardWrite permission, which limits it WebExtensions only. Would we
> also need to do a similar thing behind a clipboardRead permission to read
> the image?

Images are already translated into file objects on the clipboard (this is exposed to web content) so we won't need to add any special work to read images from the clipboard.
Duplicate of this bug: 1325759
Hi Baku, Was just curious if the latest patch is on your radar for the near future?   We'll be using if for a large China add-on when it is approved.
Flags: needinfo?(baku)
Comment on attachment 8883603 [details]
Bug 1356543 - Allow images to be copied via dataTransfer.setData

https://reviewboard.mozilla.org/r/154476/#review166212

You can also consider the using of Blobs also for strings and for streams. We have StringBlobImpl and StreamBlobImpl.
Attachment #8883603 - Flags: review?(amarchesini) → review+
Hey Rob, 

when we were looking into this, I missed the ImageBitmap [1] interface, that seems to be a newish way to standardize on passing images around in DOM land.

Sorry to spring this up on you  after you already got r+, but I think it might a good idea to standardize on that as an (only?) way of passing an image to clipboard, especially since the createImageBitmap() method [2] can convert from any other way of holding an image.

1) https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmap
2) https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/createImageBitmap
Standard discussion is not going anywhere yet.
To avoid confusion, I'm not going to piggy-back on a web platform API, and will instead add a dedicated extension API.

While doing research on the needs of the API, I saw that Chrome has a chrome.clipboard API (only for Chrome apps on Chrome OS):
https://codereview.chromium.org/2379573008
https://codereview.chromium.org/2837983002
https://developer.chrome.com/apps/clipboard

Their API support setting an image, optionally with text and/or HTML.
In order to figure out whether limiting to text and HTML only makes sense, I searched for consumers of nsITransferable's setTransferData method.
  A dumb search for "setTransferData" yielded 1999 lines in DXR addons (this includes multiple versions of add-ons).
  1250 of these have directly quoted the type (' or "), distributed as follows:
    999 text/unicode (of which 142 are part of a bundled Add-on SDK).
    166 text/html
     33 itrap/entity
     15 image/png
      9 text/plain
      7 text/xml
      6 miapps/identifier
      6 application/pp2pdf-arc-fsar
      5 text/calendar
      3 application/x-moz-file
      3 application/vnd.x-moz-cal-task
      3 application/vnd.x-moz-cal-event
      2 text/x-moz-url
      2 passwordmaker/tree
      2 passwordmaker/settings
      2 image/svg+xml
      1 text/uri-list
      1 moz/bookmarkclipboarditem
      1 application/x-chartlet-form
  348 are boilerplate from the clipboard module of the Addon SDK.
   80 are constants for "text/unicode"
   29 are constants for "text/html"
  292 other lines are not that interesting (there is no indication that any of these lines use generic MIME-types)

Based on these results, I'd say that it is safe to only support images, text and HTML.
The Add-on SDK only supports text/unicode, text/html and image/png.


I'm going to implement a clipboard API in three steps (the first two being mandatory, the third step being a nice-to-have):
Step 1: Image only (this bug)
- Implement chrome.clipboard.setImageData(ArrayBuffer, type, callback)
- API-compatible with Chrome's restricted clipboard API.
- If the image fails to encode, browser.clipboard.setImageData is rejected and the clipboard is not modified.

Step 2: Image + other types (text and html) (bug 1344410)
- Support the third "additionalItems" parameter - to allow text and HTML to be set together with the image.

Step 3: Support other types (text and html) without image
- The current API to write to the clipboard is not fully reliable, because it does not work in the background page unless OOP WebExtensions are enabled (bug 1272869).
- To support this, I will add browser.clipboard.setData(array) where the array has the same meaning as the "additionalItems" type from step 2.

Out of scope: Reading from the clipboard.
- Extensions can already read from the clipboard with document.execCommand('paste') and a contentEditable element (even in the background page - see bug 1312260, and bug 1340578 for compatibility notes).
This is rather unfortunate to need to add a *partial* and *temporary* api to extensions, but it does seem like the least bad solution. :/

Though, as this is an API design issue, a WE Peer should really confirm we want to do this, probably Kris or Shane.
(In reply to Tomislav Jovanovic :zombie from comment #33)
> Though, as this is an API design issue, a WE Peer should really confirm we
> want to do this, probably Kris or Shane.

Shane, could you review my proposed browser.clipboard API as described in comment 32?



The spec is by garyc (who works on this feature for Chrome), and his last reply was 2 months ago: https://github.com/w3c/clipboard-apis/issues/44#issuecomment-313664213. Given the many open questions, it is very unlikely for a standard API to be ready within a few months (definitely not before Firefox 57).
Flags: needinfo?(mixedpuppy)
Depends on: 1395172
Whiteboard: [outreach][moz china][triaged] → [outreach][moz china][triaged][awe:easyscreenshot@mozillaonline.com]
I consider a clipboard API important for WebExtensions, its something that we've had before and right now the current implementation is hard to get right. This isn't the perfect solution, but its manageable and understandable for our developers. Let's go with a dedicated extension API and when document it, ensure we let people know that it might change.
Attachment #8882856 - Attachment is obsolete: true
Attachment #8883603 - Attachment is obsolete: true
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review180738

The code is all fine, the only issue I really have is the permission issue.  I'd like to see this work with clipboardWrite.

::: toolkit/components/extensions/schemas/clipboard.json:18
(Diff revision 1)
> +      }
> +    ]
> +  },
> +  {
> +    "namespace": "clipboard",
> +    "description": "A temporary placeholder for a clipboard API until the web platform has a standardized clipboard manipulation API.",

The description should be about the api, not about the issues with web standards.

::: toolkit/components/extensions/schemas/clipboard.json:19
(Diff revision 1)
> +    ]
> +  },
> +  {
> +    "namespace": "clipboard",
> +    "description": "A temporary placeholder for a clipboard API until the web platform has a standardized clipboard manipulation API.",
> +    "permissions": ["clipboard"],

I'm not convinced about having a third clipboard permission.  I would be good to just use clipboardRead/Write as documented.

::: toolkit/components/extensions/schemas/clipboard.json:37
(Diff revision 1)
> +            "description": "The image data to be copied."
> +          },
> +          {
> +            "type": "string",
> +            "name": "imageType",
> +            "enum": ["jpeg", "png"],

Is it expected that addons would convert other image types?  e.g. gif.  Can the type support be broader?
Attachment #8903707 - Flags: review?(mixedpuppy)
Not sure i'll get to fully reading the api stuff this weekend.  I think we should not be concerned with Chrome compatibility when their api is restricted.  I'd like to see more image format support, at least svg and gif, unless there is a very good reason not to.  Since reading is not necessary in this api as you mentioned, lets make use of clipboardWrite per docs on mdn[1].

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Interact_with_the_clipboard
Flags: needinfo?(mixedpuppy)
these updated APIs will be in Firefox 57 Nightly (and Firefox 57 Release)?
or is it for the new Nightly branch (58,59)?
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review180738

> The description should be about the api, not about the issues with web standards.

I rephrased it, and explained what the API does (writing to the clipboard), and why reading is not supported (=because the standard web APIs can be used).

When we get a web platform API to write images (in the future), then this API's main purpose is to maintain backwards-compatibility with extensions that were written before that API existed.

> I'm not convinced about having a third clipboard permission.  I would be good to just use clipboardRead/Write as documented.

Done.

> Is it expected that addons would convert other image types?  e.g. gif.  Can the type support be broader?

I am conservative in adding support for types, to limit the maintenance burden.
In the implementation, the image is decoded first, and then exported to a platform-dependent image format.

PNG is lossless, JPEG is lossy. Both probably cover the majority of use cases (and note that the Add-on SDK only supported PNG, and there hasn't been any massive complaints about it).
Any other supported raster image can be converted via `<canvas>`.

GIF is quite often used for animation, but only one frame of the image is exported to the clipboard, so that limits the usefulness of exporting GIFs. According to https://bugzil.la/1356543#c32, there is no add-on that copies GIFs.

SVG is not supported because one who wants to export SVG expects a vector image to be exported, NOT a raster image. I am not sure yet of the security implications of exporting raw SVG as-is, and I don't want to block this feature on that. There is a twelve-year old bug on Bugzilla about copying SVG, and it does not appear to have any significant interest from others (bug 334801). And as shown in https://bugzil.la/1356543#c32, there is not much interest from add-ons to copy SVG either.
Hence, SVG is NOT part of the basic release.

I don't rule out the possibility of exporting SVG in the future, but that would be done as part of bug 1344410.
I have pushed an updated patch with the comments addressed (and a smaller JPEG image in the test case).

Try run is here (initiated locally because it depends on bug 1395172, and reviewboard does not support dependent patches AFAIK).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3a89e461c2346c827d6b132a5ded00a1ba6afff




(In reply to Oleksandr from comment #39)
> these updated APIs will be in Firefox 57 Nightly (and Firefox 57 Release)?
> or is it for the new Nightly branch (58,59)?

AFAIK anything that lands on 15 September or earlier will be in 57.
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review180820

make sure the permission string is added (e.g. bug 1394134), waiting on that and the todo question.

::: toolkit/components/extensions/schemas/clipboard.json:5
(Diff revision 2)
> +[
> +  {
> +    "namespace": "clipboard",
> +    "description": "Offers the ability to write to the clipboard. Reading is not supported because the clipboard can already be read through the standard web platform APIs.",
> +    "permissions": ["clipboardWrite"],

We might want a followup so the api works without the permission if it's a user event (as clipboardWrite otherwise works), but it's not that important.

::: toolkit/components/extensions/test/xpcshell/test_ext_clipboard_image.js:27
(Diff revision 2)
> +      ctx.drawImage(img, 0, 0);  // Draw without scaling.
> +      var pixelData = ctx.getImageData(0, 0, 1, 1).data;
> +      // expected 255, but can be 254 after image conversion from JPEG.
> +      browser.test.assertTrue(pixelData[0] >= 254, `pixel ${pixelData[0]} should be red`);
> +      // TODO: Why is the image conversion lossy even for PNG files (which is a
> +      // lossless image format)?

Will this todo be addressed?
Attachment #8903707 - Flags: review?(mixedpuppy)
No longer depends on: 1395172
Comment on attachment 8904488 [details]
Bug 1356543 - Change test_ext_clipboard_image from xpcshell to mochitest

https://reviewboard.mozilla.org/r/176172/#review181496

please just do this in the original patch that the file was introduced in.
Attachment #8904488 - Flags: review?(mixedpuppy) → review-
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review181508

r+ with removal of platform check, callback in schema and makeing the xpcshell->mochitest change from the other patch.

::: toolkit/components/extensions/ext-clipboard.js:22
(Diff revisions 2 - 3)
>      return {
>        clipboard: {
>          async setImageData(imageData, imageType) {
> +          if (AppConstants.platform == "android") {
> +            return Promise.reject({message: "Writing images to the clipboard is not supported on Android"});
> +          }

Don't include the schema in android builds (see identity) in schemas/jar.mn

::: toolkit/components/extensions/schemas/clipboard.json:14
(Diff revisions 2 - 3)
>          "type": "function",
>          "description": "Copy an image to the clipboard. The image is re-encoded before it is written to the clipboard. If the image is invalid, the clipboard is not modified.",
>          "async": "callback",
>          "parameters": [
>            {
> -            "type": "object",
> +            "type": "any",

Why was this change necessary?

::: toolkit/components/extensions/schemas/clipboard.json:11
(Diff revision 3)
> +    "functions": [
> +      {
> +        "name": "setImageData",
> +        "type": "function",
> +        "description": "Copy an image to the clipboard. The image is re-encoded before it is written to the clipboard. If the image is invalid, the clipboard is not modified.",
> +        "async": "callback",

no callback with new APIs.  Chrome's API is not public, so it doesn't matter to be compatible here.
Attachment #8903707 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review181508

> Why was this change necessary?

The test failed on debug mode because of non-compliance with the schema and I tried to fix it. Changing to "any" still does not work, I read the code in Schemas.jsm more closely and reverted back to `"type":"object"` PLUS `"additionalProperties": true`.

> no callback with new APIs.  Chrome's API is not public, so it doesn't matter to be compatible here.

Chrome's API is public, it can be used by extension on Chrome OS.
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review181508

> Don't include the schema in android builds (see identity) in schemas/jar.mn

I included the schema because in the follow-up the `clipboard` API is going to be extended with putting text and HTML on the clipboard, and writing text to the clipboard is supported on Android.
just merge the patches, I’m fine with the android check given the reason.

Chrome's api is an experimental app api for Chrome OS and only on dev channel[1], so not really public.  We're going to go beyond what they support and we're going to release with it.  By relying on sharing the clipboardWrite permission, we're already breaking that compat.  

So...if the api is indeed available to extensions on release versions of the chrome web browser, then I'd revert some of my review to be sure we're indeed compatible on this (including the permission change).  But if it isn't, I see no reason for a pretense on compatibility so just do away with the callback.

[1] https://developer.chrome.com/apps/clipboard
Attachment #8904488 - Attachment is obsolete: true
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review181540

::: toolkit/components/extensions/schemas/clipboard.json
(Diff revisions 3 - 4)
>      "functions": [
>        {
>          "name": "setImageData",
>          "type": "function",
>          "description": "Copy an image to the clipboard. The image is re-encoded before it is written to the clipboard. If the image is invalid, the clipboard is not modified.",
> -        "async": "callback",

You need to keep "async": true, and remove the callback parameter.
Comment on attachment 8903707 [details]
Bug 1356543 - Add clipboard.setImageData API

https://reviewboard.mozilla.org/r/175480/#review181546

::: toolkit/components/extensions/schemas/clipboard.json:27
(Diff revisions 4 - 5)
>              "name": "imageType",
>              "enum": ["jpeg", "png"],
>              "description": "The type of imageData."
>            },
>            {
>              "name": "callback",

You still didn't remove the callback parameter.
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: waiting for lock on working directory of /repo/hg/mozilla/try held by process '2253' on host 'hgssh4.dmz.scl3.mozilla.com/effffffc'
remote: abort: working directory of /repo/hg/mozilla/try: timed out waiting for lock held by 'hgssh4.dmz.scl3.mozilla.com/effffffc:2253'
abort: stream ended unexpectedly (got 0 bytes, expected 4)
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/61feaf98e908
Add clipboard.setImageData API r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/61feaf98e908
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Blocks: 1397690
Flags: needinfo?(baku)
Copying images to the clipboard is now supported in Firefox 57.

For documentation, looking at the patches should get you far.
This implementation is just step 1, the setImageData API will also be extended with another parameter, see
https://bugzilla.mozilla.org/show_bug.cgi?id=1356543#c32 (I also suggest to take a look at that comment if you write documentation).
Keywords: dev-doc-needed
Is any manual testing necessary around this bug? If yes, could you please provide some reliable steps? Is there any webextension to verify this fix?
Firefox 57.0a1 (2017-09-15)

in manifest.json
"permissions": [
	"clipboardWrite"
],

in JavaScript this code from PasteBin:
https://pastebin.com/sgXPLZs0

I already checked - everything works fine, thanks Mozilla! :)
PS: 
you can use this documentation: https://developer.chrome.com/apps/clipboard
but in Mozilla not work callback and additionalItems:
chrome.clipboard.setImageData(ArrayBuffer imageData, enum of "png", or "jpeg" type, array of object additionalItems, function callback)
PPS:
> but in Mozilla not work callback and additionalItems

chrome.clipboard.setImageData(ab, 'png', function() {
	chrome.notifications.create({ 'type' : 'basic', 'title': 'copy', 'message' : 'OK!' });
});

Error: Incorrect argument types for clipboard.setImageData.
@Oleksandr
The second parameter will be supported as part of bug 1344410.

While your snippet (comment 61) works, there is a simpler way to convert from a base64-encoded image to an ArrayBuffer:
https://searchfox.org/mozilla-central/rev/2c9a5993ac40ec1db8450e3e0a85702fa291b9e2/toolkit/components/extensions/test/mochitest/test_ext_clipboard_image.html#144-147

And if you have a Blob, then you can do this:

// Assume that you have a blob variable, an instance of Blob.
var fr = new FileReader();
fr.onloadend = async function() {
  var arraybuffer = fr.result;
  // Note: If the Blob is invalid, fr.result can be null.
  try {
    await browser.clipboard.setImageData(arraybuffer, "png");
  } catch (e) {
    console.log("Failed to copy image: " + e.message);
    return;
  }
  console.log("Image successfully copied");
};
fr.readAsArrayBuffer(blob);


There are other differences between Chrome and Firefox:

Firefox only requires clipboardWrite.
Chrome requires clipboard and clipboardWrite.
> there is a simpler way to convert from a base64-encoded image to an ArrayBuffer:

let imageData = Uint8Array.from(atob(b64data), c => c.charCodeAt(0)).buffer;
Thank you! :)

> The second parameter will be supported as part of bug 1344410.

setImageData will support callback? in Firefox Nightly 57?

> Chrome requires clipboard and clipboardWrite.

I want to pay attention - it is supported only for Chrome Apps. 
But Chrome apps will die in 2017:
https://blog.chromium.org/2016/08/from-chrome-apps-to-web.html
>> there is a simpler way to convert from a base64-encoded image to an ArrayBuffer:
> let imageData = Uint8Array.from(atob(b64data), c => c.charCodeAt(0)).buffer;
> Thank you! :)

Sorry... But...
========
Your code:
========
var dd = (new Date()).getTime();
	var imageData = Uint8Array.from(byteString, c => c.charCodeAt(0)).buffer;
var dd2 = (new Date()).getTime();
console.log(dd2-dd);
========
Result:
1. 25ms
2. 20ms
3. 18ms
4. 21ms
5. 17ms

========
My code:
========
var dd = (new Date()).getTime();
	var ab = new ArrayBuffer(byteString.length);
	var ia = new Uint8Array(ab);
	for (var i = 0; i < byteString.length; i++) {
		ia[i] = byteString.charCodeAt(i);
	}
var dd2 = (new Date()).getTime();
console.log(dd2-dd);
========
Result:
1. 4ms
2. 0ms
3. 2ms
4. 0ms
5. 3ms
PS: 

> Sorry... But...
> Your code:

I tested for image = 100 kilobytes
Note for documentation: the clipboard API should also be referenced at
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Comparison_with_XUL_XPCOM_extensions#Privileged_APIs
Could you also reference the clipboard API from
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Interact_with_the_clipboard
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Comparison_with_XUL_XPCOM_extensions#Privileged_APIs

And add the note that the API is also defined in Chrome, but **only on Chrome OS** (on other platforms an error would be displayed, see e.g. https://github.com/mozilla/webextension-polyfill/issues/70 ).
(Chrome compat is currently mentioned in the intro of setImageData, but not present in the compat table; please add it too).


For the unit tests I used a base64-encoded string to keep the test self-contained, for documentation it is probably better to show how to copy a given URL:

// requires "https://cdn.mdn.mozilla.net/*", "clipboardWrite"
fetch('https://cdn.mdn.mozilla.net/static/img/favicon144.png')
.then(response => response.arrayBuffer())
.then(buffer => browser.clipboard.setImageData(buffer, 'png'));

Or an image bundled with the extension:

// requires "clipboardWrite"
fetch(browser.runtime.getURL('image.png'))
.then(response => response.arrayBuffer())
.then(buffer => browser.clipboard.setImageData(buffer, 'png'));
Flags: needinfo?(rob)
Thanks for the feedback Rob!

(In reply to Rob Wu [:robwu] from comment #70)
> Could you also reference the clipboard API from
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/
> Interact_with_the_clipboard

I forgot about this.

> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/
> Comparison_with_XUL_XPCOM_extensions#Privileged_APIs

I added it here already, under nsIClipboard, unless you mean something else. 

> And add the note that the API is also defined in Chrome, but **only on
> Chrome OS** (on other platforms an error would be displayed, see e.g.
> https://github.com/mozilla/webextension-polyfill/issues/70 ).
> (Chrome compat is currently mentioned in the intro of setImageData, but not
> present in the compat table; please add it too).

I don't think it makes sense to add this to the compat data for Chrome. The compat data is for extensions, not apps, and AIUI this API is not supported for extensions. I do think it's worth mentioning in the preamble that the API is based on the Chrome API though.

> For the unit tests I used a base64-encoded string to keep the test
> self-contained, for documentation it is probably better to show how to copy
> a given URL:
> 
> // requires "https://cdn.mdn.mozilla.net/*", "clipboardWrite"
> fetch('https://cdn.mdn.mozilla.net/static/img/favicon144.png')
> .then(response => response.arrayBuffer())
> .then(buffer => browser.clipboard.setImageData(buffer, 'png'));
> 
> Or an image bundled with the extension:
> 
> // requires "clipboardWrite"
> fetch(browser.runtime.getURL('image.png'))
> .then(response => response.arrayBuffer())
> .then(buffer => browser.clipboard.setImageData(buffer, 'png'));


Much better examples, thanks!
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(rob)
Already covered by automated tests.
Flags: needinfo?(rob) → qe-verify-
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.