407983 - Add support clipboardData object for the onpaste, oncopy, oncut events

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[spam]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121108 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121108 Minefield/3.0b2pre

There is currently no way of retrieving the clipboard data in JavaScript, WebKit and IE both have methods for this but Gecko currently lacks this functionality.

WebKit uses a method of attaching the an clipboardData object to the onpaste event. This method is better since it only gives JS access to the clipboard contents when the user performs an paste action.

This is how WebKit does it, contents can be retrieved from the onpaste event object:
eventObj.clipboardData.getData('text/plain');

While in IE you can use, this enables to to retrieve the contents at any time:
window.clipboardData.getData('Text');

IE and WebKit have only the ability to return plain text contents ideal would be to have the ability to get the rich HTML contents to be used in Midas/designMode implementations.

Reproducible: Always

Comment 1

[Martijn Wargers (dead)]

Could you perhaps attach example code of how this is working in IE and Safari?
Also, giving the urls that reference this behavior in IE and Safari would be nice.

Comment 2

[spam]

Attachment: Testcase for the clipboardData object in IE and WebKit

This attachment displays how the plaintext contents can be extracted in IE and WebKit using the clipboardData object.

Comment 3

[spam]

Here are some references of the IE and WebKit implementations:

http://msdn2.microsoft.com/en-us/library/ms535220.aspx
http://developer.apple.com/documentation/AppleApplications/Reference/SafariJSRef/Classes/Events.html

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Has anyone studied the possible security problems clipboardData may expose?
Do IE or Safari somehow limit the use of .clipboardData? (I do hope so)
The documentation isn't very clear.

Comment 5

[spam]

IE will popup an confirm dialog before you can extract anything from the clipboard at least IE7. Safari will only provide the clipboardData object on a paste event so it will only be available if the user actually pastes some contents into the browser window so both methods are pretty secure.

Comment 6

[Dan POPA]

I agree this is secure.


In IE6 you can change the security to: disable clipboard access, enable or prompt. By default in IE6 is enabled. If prompt, a nice prompt asks the user permission for the clipboard operation. In IE7 default is prompt.

In Safari: access to clipboardData.getData(flavor) is given only on the paste event inside the onpaste event handlers. So only an user initiated event can exposes clipboard contents to the underlying application.



If a malicious script somehow would get to listen to the onpaste event to steal/alter the clipboard then it would certainly be able to do all sorts of stuff, much worse than this. It would not even need to get the clipboard, it would redirect the form submission itself.

If a script can listen to the onpaste event then it certainly can still your form data.

Comment 7

[Jeff Walden [:Waldo]]

Odds are I can't clear the flag, but given that onpaste and friends are being backed out, this doesn't block.

Comment 8

[Jeff Walden [:Waldo]]

Blah, I suck -- it's only the onbefore* versions that are being removed.  Restoring blocking request...

Comment 9

[Damon Sicore (:damons)]

New support this late?  -'ing.

And, can you explain why you are requesting blocking again?

Comment 10

[Jeff Walden [:Waldo]]

Dan, can you answer that?

(Note I was just restoring a flag I accidentally removed, not seriously requesting blocking here!)

Comment 11

[Dan POPA]

(In reply to comment #10)
> Dan, can you answer that?
> (Note I was just restoring a flag I accidentally removed, not seriously
> requesting blocking here!)

The clipboardData may be used toghether with the contentEditable attribute.

Application logic may require that the onpaste and ondrop events to be stopped and the clipboard content be retrieved so it can be cleaned/transformed/whatever and then inserted into the document.

It would be nice to have this in FF3 timeframe so apps can implement cross-browser custom paste handlers.

Comment 12

[Damon Sicore (:damons)]

This is something we probably wouldn't even take for 1.9 at this point in the release cycle (after confirming with jst).  We should have the discussion of whether or not this blocks the next release (Moz 2).  Marking wanted-next for now.

Comment 13

[Danny Bloemendaal]

What is the status for this issue?
Imho it's kinda silly that you implement a paste event but don't give a handle to what is actually being pasted.

(also, why is this more a security issue that the keydown/press/up event? if you want to do bad things that that's the event I would spy on. Not paste probably).

Comment 14

[Danny Bloemendaal]

Actually.. I don't need access to the clipboard at all. Just as long as the event passes me the string of what it wants to paste (for my to change (read: remove Word markup ****)).

Comment 15

[Dan POPA]

That would work for me too: tell me what its about to be pasted and let me change it, only for the paste event, thank you.

This may be text content when pasting into form controls and a HTML string when pasting into contenteditable elements.

Comment 16

[Dion Almaer]

I am excited to see this support added as it would mean that we could enable a full copy/cut/paste experience for Bespin (bespin.mozilla.com).

We currently have it working great in WebKit, but are stuck with poor support in Firefox (the buffer is not the system clipboard, but rather simply within the app itself).

Comment 17

[spam]

As you have a dataTransfer object on the drop event now in the latest nightly it should be a easy fix to add this to the paste event as well. According to the HTML 5 spec a paste operation should fire a drop event. However I would prefer to have this dataTransfer object to the specific paste event as suggested in this feature request.

http://www.whatwg.org/specs/web-apps/current-work/#copy-and-paste

Comment 18

[Neil Deakin]

The basics of this are reasonably simple to implement. Just add a pointer to the dataTransfer in the cut/copy/paste events, then hook up the mappings to the transferables. We already support the events.

We should not implement any clipboard operations through drag events. The 'spec' is absurd in this regard.

Comment 19

[Eli Grey (:sephr)]

(In reply to comment #18)
> We should not implement any clipboard operations through drag events. The
> 'spec' is absurd in this regard.

In the same regard to how paste is metaphorically just an instant drag from the system clipboard, copy would be the inverse. In my opinion, it doesn't make sense to only support paste/drop but not copy/drag.

Comment 20

[spam]

Does anyone know if there is a master bug for the new drag/drop feature added in FF 3.5? Since this is somewhat related I want to attach this bug as a dependency if there is one.

I understand that the drop event is basically the same as a paste event but that doesn't stop the possibility to have the dataTransfer object added both to the paste and drop event. And firing the drop event on a paste operation would make it compatible with the HTML 5 spec. Then one could choose to use the paste event or the drop event if you want to distinct the two.

Comment 21

[Alan Tam]

Just installed Firefox 3.5 today. Simply cannot imagine why clipboardData is still left out. The "need discussion" resolution mentioned in Comment 12 is already 15 months old. It has not given concrete reasons why it can only be implemented in Gecko 2.0 timeframe the earliest.

Comment 18 also mentioned that it is fairly easy to implement. The only concern is perhaps that "we should not implement any clipboard operations through drag event". However, as Comment 20 stated, we only need to make sure the dataTransfer object is added to both events, without really need to merge the code for handling the events.

So, what are we still waiting for?

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #21)
> So, what are we still waiting for?

Someone doing the work. (Bug 501154 looks promising.)

Comment 23

[Neil Deakin]

Attachment: work in progress

This patch implements event.clipboardData and cuts/copies any data set into it onto the clipboard, as well as fills in the clipboard data on paste.

It doesn't implement window.clipboardData, nor the beforeXXX events.

Comment 24

[Neil Deakin]

Builds are at https://build.mozilla.org/tryserver-builds/[e-mail]-clipboard4/

Comment 25

[Alan Tam]

The build seems lost. Anywhere to try it out now?

Comment 26

[Jesse Ruderman]

Please add tests to ensure web pages can only use getData/setData *during* the appropriate event.  For example, they shouldn't be able to save off an event or clipboardData object from a copy event, and then later call setData successfully.

Comment 27

[Tobias]

Hi there, I am new here so this may be the wrong place for this request:

I really dont understand why browser dont allow me to allow it to paste images into an rich-text-editor or whatever enabled UI element in the browser. 
Think about how much easier blogging would be if you just could paste the screenshot you just made or the imagefile you just copied from your desktop to the RTE and go on editing it (with the awesome html5- or flash image editors).
I know there is some security issue with that. But we already have solutions for this, dont we? I dont think every site should be allowed to access the imanges in my clipboard. It would be enough for me to whitelist the sites like firefox already does with plugin-installation-sites or with java-applets that request for HDD access or something.
In my opinion it cannot be that it is easier for me to make a screencast of my system with screenr.com that to add an image to my blog-article.

What do you thinks?

Comment 28

[Stéphane Roucheray]

This must be implemented in order for Bespin to work properly as a text editor.

Where are we for now ?

Comment 29

[Alan Tam]

Now that bug 501154 is fixed, I guess we only need to wait for Neil Deakin to update his patch.

Comment 30

[Tobias]

@AlanSiu-LungTam: Fixed in which way? Will I be able to paste images from my system into the browser for further use? (See https://bugzilla.mozilla.org/show_bug.cgi?id=407983#c27) -- Or should I open a different ticket for that?
Thanks

Comment 31

[Alan Tam]

Tobias,

Once this bug is fixed, hopefully your requested feature can be implementable by javascript that captures the whole image data via clipboardData in the onpaste event. However, I am not 100% it will work flawlessly, since webkit based browsers currently allow reading texts from clipboard but not images.

Comment 32

[Tobias]

Thanks Alan!
I created a new request 570460 that describes in detail what I am hoping to have some day: https://bugzilla.mozilla.org/show_bug.cgi?id=570460

Maybe some of you here can have a look at the ticket – it is my first one so I hope you will put it right if I did anything wrong. Thanks!

Comment 33

[Neil Deakin]

Attachment: patch part 1 - add clipboardData to cut/copy/paste events

Comment 34

[Neil Deakin]

Attachment: Patch part 2 - global clipboardData property

This patch adds a window.clipboardData property as IE does for clipboard access outside a clipboard event. Probably should be defined on nsIDOMChromeWindow, but I wrote this a while ago and didn't want to change it now.

In any case, security checks are made for non-chrome to ensure that access is only granted during clipboard events. Chrome callers have access at any time.

There's also a window.selectionClipboardData but I'm not sure how useful that really is to customize.

Comment 35

[Neil Deakin]

Attachment: Patch part 3 - clipboard changed checks

This patch adds an optimization such that the global clipboard data is only re-retrieved if the clipboard has changed.

I think this only works on gtk when data on the clipboard comes from us, not from other applications, so an additional check will need to be added.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

How close is the implementation to http://dev.w3.org/2006/webapi/clipops/ ?
(I haven't reviewed the draft spec )

Comment 37

[Neil Deakin]

This spec seems a bit vague in places so its hard to tell, but it looks to be the same. Some differences:

- 4.1 bullet 3 about pasting images and html isn't done, but that doesn't seem specific to clipboard usage and should just apply to data transfers in general. My patch populates the data transfer using the same code as with drag and drop.
- the clipboard event interface uses a type/data, whereas I use a DataTransfer, which is more consistent with the drag/drop events
- this spec seems to imply that access to the system clipboard is live. My implementation doesn't have a live version used for event.clipboardData during an event. The clipboard is only modified after the cut/copy event has finished. This spec is inconsistent though, in some places in says a live clipboard is used, but in another (4.1 bullet 5) it says that the clipboard is only modified if the default action is prevented, which is only known after the event finishes.
- this spec seems to want clipboard events fired on keydown, whereas we use keypress. Not clear why this spec is specifying this, since the manner in which keyboard shortcuts are handled seems browser/platform specific
- I don't attempt to process, filter or perform any special security checks on the data, beyond what the data transfer and existing clipboard code already does.

Comment 39

[Fabien Cazenave [:kaze]]

FWIW, I’ve started rebasing Neil’s patches: https://tbpl.mozilla.org/?tree=Try&rev=5e527f465905

Seems to work here, now looking more closely at the spec.

Comment 40

[Fabien Cazenave [:kaze]]

Attachment: patch part 1 - add clipboardData to cut/copy/paste events

Comment 41

[Fabien Cazenave [:kaze]]

Attachment: patch part 2 - global clipboardData property

Comment 42

[Fabien Cazenave [:kaze]]

Attachment: patch part 3 - clipboard changed checks

Comment 43

[(no longer active)]

Fabien, what is the status of these patches?

Comment 44

[Rob Campbell [:rc] (:robcee)]

pinging for a status update on this.

The fix for this bug would make our Source Editor integration with Orion much better. Also, this would allow the Orion team to remove some fairly ugly hacks from their editor implementation for Firefox. If it would be good for them, it would be good for the rest of the web.

Comment 45

[Dmitry Alexeenko [MSFT]]

Guys, is there any update on this issue by chance?

Comment 46

[(no longer active)]

Neil, do you think you'll have a chance to work on this in the near future?

Comment 47

[Neil Deakin]

I can provide updated patches, if someone would like to volunteer to provide feedback on them. Comment 37 probably still applies, but I can review the updated spec again, (or someone else can)

Comment 48

[(no longer active)]

I can try to provide some feedback for you.  :-)

Comment 49

[Neil Deakin]

Attachment: Part 1 - add clipboardData to cut/copy/paste events

Comment 50

[Neil Deakin]

Attachment: Part 2 - global clipboardData property

See comment 34 for info about this part.

Comment 51

[Neil Deakin]

Attachment: Part 3 - clipboard changed checks

This is really only implemented on Windows and Mac.

Comment 52

[Dmitry Alexeenko [MSFT]]

Just wondering if you guys have some approx. timeline in mind for this fix?

Also - will clipboardData contain DataTransferItemList, so clipboardData users can look through it to see what kind of content is in clipboardData?

Comment 53

[Neil Deakin]

(In reply to Dmitry Alexeenko [MSFT] from comment #52)
> Also - will clipboardData contain DataTransferItemList, so clipboardData
> users can look through it to see what kind of content is in clipboardData?

It will not. But you can find out what kind of content is there with clipboardData.types.

Comment 54

[Steve Lancashire]

Is this still being worked on? Any chance of an update?  Is there a target milestone for this?

Comment 55

[(no longer active)]

Comment on attachment 633602 [details] [diff] [review]
Part 1 - add clipboardData to cut/copy/paste events

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

Looks good overall, although I did not do an in depth review.

::: content/base/src/nsCopySupport.cpp
@@ +706,5 @@
>    if (!nsContentUtils::IsSafeToRunScript())
>      return false;
>  
> +  // next, fire the cut, copy or paste event
> +  // XXXndeakin why was a preference added here without a running-in-chrome check?

Worth filing a bug for!

::: content/events/src/nsDOMClipboardEvent.cpp
@@ +104,5 @@
> +{
> +  nsClipboardEvent* event = static_cast<nsClipboardEvent*>(mEvent);
> +
> +  if (!mEventIsInternal && !event->clipboardData) {
> +    // XXXndeakin is this ever called?

Hmm, this should not happen...  Why not MOZ_ASSERT that?

@@ +121,5 @@
> +  nsDOMClipboardEvent* it =
> +    new nsDOMClipboardEvent(aPresContext, aEvent);
> +  if (nsnull == it) {
> +    return NS_ERROR_OUT_OF_MEMORY;
> +  }

Nit: the null check is unnecessary.

::: content/events/src/nsDOMDataTransfer.cpp
@@ +976,5 @@
> +  NS_ASSERTION(mEventType != NS_CUT && mEventType != NS_COPY,
> +               "clipboard event with empty data");
> +
> +  nsCOMPtr<nsITransferable> trans =
> +    do_CreateInstance("@mozilla.org/widget/transferable;1");

Note that you need to init this transferable.

::: content/events/src/nsDOMEvent.cpp
@@ +702,5 @@
> +    case NS_CLIPBOARD_EVENT:
> +    {
> +      nsClipboardEvent* oldClipboardEvent = static_cast<nsClipboardEvent*>(mEvent);
> +      nsClipboardEvent* clipboardEvent = new nsClipboardEvent(false, msg);
> +      NS_ENSURE_TRUE(clipboardEvent, NS_ERROR_OUT_OF_MEMORY);

clipboardEvent will never be null.

Comment 56

[(no longer active)]

Comment on attachment 633603 [details] [diff] [review]
Part 2 - global clipboardData property

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

Why do we want to implement the global clipboardData property?

Comment 57

[(no longer active)]

(In reply to Ehsan Akhgari [:ehsan] from comment #56)
> Comment on attachment 633603 [details] [diff] [review]
> Part 2 - global clipboardData property
> 
> Review of attachment 633603 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why do we want to implement the global clipboardData property?

To be clear, I did read comment 34 (and also you blog post here http://enndeakin.wordpress.com/2010/03/04/clipboard-data/) but I am not convinced that this is a good idea.  For usage within the Mozilla applications, chrome code can continue to use nsIClipboard.

Comment 58

[(no longer active)]

Comment on attachment 633604 [details] [diff] [review]
Part 3 - clipboard changed checks

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

This looks mostly good as well.

Again, so sorry that it me so long to get back to you on this.

::: widget/qt/nsClipboard.cpp
@@ +563,5 @@
> +  }
> +  else {
> +    *aChanged = false;
> +  }
> +  return NS_OK;

Perhaps return NS_ERROR_NOT_IMPLEMENTED?

Comment 59

[Julian Viereck]

This would be good to have for PDF.JS when copy selected text.

How much is required to make this land? Can we move the "global clipboardData property" decision to another bug and make the "Part 1" and "Part 3" land?

Comment 60

[(no longer active)]

(In reply to comment #59)
> This would be good to have for PDF.JS when copy selected text.
> 
> How much is required to make this land? Can we move the "global clipboardData
> property" decision to another bug and make the "Part 1" and "Part 3" land?

We just need someone to finish up the patches here.  Neil, do you have time to do that?

Comment 61

[Neil Deakin]

(In reply to Ehsan Akhgari [:ehsan] from comment #57)

> To be clear, I did read comment 34 (and also you blog post here
> http://enndeakin.wordpress.com/2010/03/04/clipboard-data/) but I am not
> convinced that this is a good idea.  For usage within the Mozilla
> applications, chrome code can continue to use nsIClipboard.

The intent is to remove/deprecate it and nsITransferable in favour of nsIDOMDataTransfer to reduce duplicate and complicated code. That said, I could just add a clipboardData property to nsIClipboard instead.

Comment 62

[Neil Deakin]

(In reply to Julian Viereck from comment #59)
> How much is required to make this land? Can we move the "global
> clipboardData property" decision to another bug and make the "Part 1" and
> "Part 3" land?

Without Part 2, part 3 isn't needed. Part 3 doesn't yet work properly on Linux as described in comment 35 and I don't know how to fix it.

Comment 63

[(no longer active)]

I think it's worth trying to land the first part here which is going to provide a lot of value to web developers, and move parts 2 and 3 to another bug and try to work on them to get them landed separately.  Do you agree?  If yes, it would be great if you can address my previous comments on part 1 and submit a patch for review.  Thanks!

Comment 64

[Neil Deakin]

Attachment: Latest patch

OK, here is the latest patch that addresses the feedback above.

Comment 65

[Julian Viereck]

(In reply to Neil Deakin from comment #64)
> Created attachment 673950 [details] [diff] [review]
> Latest patch
> 
> OK, here is the latest patch that addresses the feedback above.

Awesome! Do you want to setup someone for review on the patch?

Comment 66

[(no longer active)]

(In reply to comment #65)
> (In reply to Neil Deakin from comment #64)
> > Created attachment 673950 [details] [diff] [review]
> > Latest patch
> > 
> > OK, here is the latest patch that addresses the feedback above.
> 
> Awesome! Do you want to setup someone for review on the patch?

I can do the review if the patch is ready for review, Neil.

Comment 67

[Neil Deakin]

Comment on attachment 673950 [details] [diff] [review]
Latest patch

You can review it if you like. The spec pointed to in comment 36 appears to have changed quite a bit since then so this patch likely matches it less than I mentioned earlier.

Comment 68

[(no longer active)]

Comment on attachment 673950 [details] [diff] [review]
Latest patch

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

Apologies for the delay.  r=me with the below addressed.

::: content/base/src/nsCopySupport.cpp
@@ +709,5 @@
>    if (!nsContentUtils::IsSafeToRunScript())
>      return false;
>  
> +  // next, fire the cut, copy or paste event
> +  // XXXndeakin why was a preference added here without a running-in-chrome check?

Please file a follow-up on this, thanks!

@@ +716,4 @@
>    if (Preferences::GetBool("dom.event.clipboardevents.enabled", true)) {
> +    clipboardData = new nsDOMDataTransfer(aType, aType == NS_PASTE);
> +    if (!clipboardData)
> +      return false;

No need for the null check.

::: content/events/src/nsDOMClipboardEvent.h
@@ +33,5 @@
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the MPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */
> +

Nit: please use the MPL2 header in the new files.

::: content/events/src/nsDOMDataTransfer.cpp
@@ +223,5 @@
>  {
>    *aFileList = nullptr;
>  
> +  if (mEventType != NS_DRAGDROP_DROP && mEventType != NS_DRAGDROP_DRAGDROP &&
> +      mEventType != NS_PASTE)

Hmm, this seems wrong.  I think you're missing a return statement.

@@ +688,5 @@
> +                                   nsITransferable** aTransferable)
> +{
> +  *aTransferable = nullptr;
> +
> +  nsTArray<TransferItem>& item = mItems[aIndex];

As a precaution against someone modifying |item| by accident in the future, please declare it as const.

@@ +696,5 @@
> +
> +  nsCOMPtr<nsITransferable> transferable =
> +    do_CreateInstance("@mozilla.org/widget/transferable;1");
> +  if (!transferable)
> +    return;

Hmm, I think do_CreateInstance should be infallible these days... But I'm not sure...

@@ +701,5 @@
> +  transferable->Init(aLoadContext);
> +
> +  bool added = false;
> +  for (uint32_t f = 0; f < count; f++) {
> +    TransferItem& formatitem = item[f];

Ditto on const-ness.

@@ +956,5 @@
> +  // there isn't a way to get a list of the formats that might be available on
> +  // all platforms, so just check for the types that can actually be imported
> +  const char* formats[] = { kFileMime, kHTMLMime, kURLMime, kURLDataMime, kUnicodeMime };
> +
> +  for (uint32_t f = 0; f < NS_ARRAY_LENGTH(formats); f++) {

Nit: Please use mozilla::ArrayLength.

@@ +1000,5 @@
> +    do_CreateInstance("@mozilla.org/widget/transferable;1");
> +  if (!trans)
> +    return;
> +
> +  trans->Init(nullptr);

I think it's a good idea to initialize this transferable with the document's load context for consistency.

::: content/events/src/nsDOMDataTransfer.h
@@ +82,5 @@
> +  // paste or a drag that was started without using a data transfer. The
> +  // latter will occur when an external drag occurs, that is, a drag where the
> +  // source is another application, or a drag is started by calling the drag
> +  // service directly.
> +  nsDOMDataTransfer(uint32_t aEventType, bool aIsExternal);

Hmm, please use an enum instead of a bool here, so that the callers look like:

  new nsDOMDataTransfer(foo, nsDOMDataTransfer::external/internal);

@@ +101,5 @@
>                          nsIDOMNode* aDragTarget);
>  
> +  // converts the data for a single item at aIndex into an nsITransferable object.
> +  void GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext,
> +                       nsITransferable** aTransferable);

Please return an already_AddRefed.

::: dom/interfaces/events/nsIDOMClipboardEvent.idl
@@ +53,5 @@
> +  void initClipboardEventNS(in DOMString namespaceURIArg,
> +                            in DOMString typeArg,
> +                            in boolean canBubbleArg,
> +                            in boolean cancelableArg,
> +                            in nsIDOMDataTransfer aClipboardData);

None of these two methods is mentioned in the spec any more, so we should be able to remove them both.

Comment 69

[Neil Deakin]

> > +    do_CreateInstance("@mozilla.org/widget/transferable;1");
> > +  if (!trans)
> > +    return;
> > +
> > +  trans->Init(nullptr);
> 
> I think it's a good idea to initialize this transferable with the document's
> load context for consistency.
> 

The document is always null here.


> Hmm, please use an enum instead of a bool here, so that the callers look
> like:
> 
>   new nsDOMDataTransfer(foo, nsDOMDataTransfer::external/internal);

Blech!

Comment 70

[Julian Viereck]

Any updates on this one?

Comment 71

[Julian Viereck]

Attachment: Rebased patch and address some review comments

This is a rebased version of "673950: Latest patch". I don't really understand the code Neil did and therefore "just" made the patches apply. It didn't loooked like the code didn't apply because there were a lot of changes, so this might be all good.

I've also tried to address the review comments made by :ehsan in comment #68. Some notes there:

> ::: content/base/src/nsCopySupport.cpp
> @@ +709,5 @@
> >    if (!nsContentUtils::IsSafeToRunScript())
> >      return false;
> >  
> > +  // next, fire the cut, copy or paste event
> > +  // XXXndeakin why was a preference added here without a running-in-chrome check?
> 
> Please file a follow-up on this, thanks!

I didn't do this one. @Neil, have you done this already/could you please do this?

> ::: content/events/src/nsDOMDataTransfer.cpp
> @@ +223,5 @@
> >  {
> >    *aFileList = nullptr;
> >  
> > +  if (mEventType != NS_DRAGDROP_DROP && mEventType != NS_DRAGDROP_DRAGDROP &&
> > +      mEventType != NS_PASTE)
> 
> Hmm, this seems wrong.  I think you're missing a return statement.

I've just placed a "return NS_OK" but have no clue if that's the right things to do there. Please double check!

> ::: content/events/src/nsDOMDataTransfer.h
> @@ +82,5 @@
> > +  // paste or a drag that was started without using a data transfer. The
> > +  // latter will occur when an external drag occurs, that is, a drag where the
> > +  // source is another application, or a drag is started by calling the drag
> > +  // service directly.
> > +  nsDOMDataTransfer(uint32_t aEventType, bool aIsExternal);
> 
> Hmm, please use an enum instead of a bool here, so that the callers look
> like:
> 
>   new nsDOMDataTransfer(foo, nsDOMDataTransfer::external/internal);

Can you please give me a pointer on how to implement this? I guess it should be done using an enum, but then I don't know how to make it be on the |nsDOMDataTransfer| directly. Should the enum be named something like |nsDOMDataTransferType| instead?

> @@ +101,5 @@
> >                          nsIDOMNode* aDragTarget);
> >  
> > +  // converts the data for a single item at aIndex into an nsITransferable object.
> > +  void GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext,
> > +                       nsITransferable** aTransferable);
> 
> Please return an already_AddRefed.

I don't understand this one. The function |GetTransferable| returns a void and already_AddRefed expects a pointer, right?

Comment 72

[Josh Matthews [:jdm]]

>> @@ +101,5 @@
>> >                          nsIDOMNode* aDragTarget);
>> >  
>> > +  // converts the data for a single item at aIndex into an nsITransferable object.
>> > +  void GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext,
>> > +                       nsITransferable** aTransferable);
>> 
>> Please return an already_AddRefed.
>
>I don't understand this one. The function |GetTransferable| returns a void and 
>already_AddRefed expects a pointer, right?

For non-XPCOM functions, it's preferable to return an already_AddRefed pointer (instead of void) and avoid using outparams.

Comment 73

[Julian Viereck]

Attachment: Updated testcase that uses onCut and onCopy as well.

Comment 74

[Julian Viereck]

Attachment: Updated testcase that uses onCut and onCopy as well. [bugfix]

Same as in "711262: Updated testcase that uses onCut and onCopy as well.", but get the string right when copy/cut the selection.

Comment 75

[Julian Viereck]

Attachment: Rebase v2 with already_AddRefed

(In reply to Josh Matthews [:jdm] from comment #72)
> >> @@ +101,5 @@
> >> >                          nsIDOMNode* aDragTarget);
> >> >  
> >> > +  // converts the data for a single item at aIndex into an nsITransferable object.
> >> > +  void GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext,
> >> > +                       nsITransferable** aTransferable);
> >> 
> >> Please return an already_AddRefed.
> >
> >I don't understand this one. The function |GetTransferable| returns a void and 
> >already_AddRefed expects a pointer, right?
> 
> For non-XPCOM functions, it's preferable to return an already_AddRefed
> pointer (instead of void) and avoid using outparams.

This patch moves the outparams as a return value. Thanks Josh for that hint! 

The other open questions from attachment #711261 [details] [diff] [review] still apply. 

I've changed the already existing GetTransferables function as well to return an already_AddRefed and had to do a new change:

+++ b/content/events/src/nsEventStateManager.cpp
@@ -2312,18 +2313,17 @@
     action = nsIDragService::DRAGDROP_ACTION_COPY |
              nsIDragService::DRAGDROP_ACTION_MOVE |
              nsIDragService::DRAGDROP_ACTION_LINK;
 
   // get any custom drag image that was set
   int32_t imageX, imageY;
   nsIDOMElement* dragImage = aDataTransfer->GetDragImage(&imageX, &imageY);
 
-  nsCOMPtr<nsISupportsArray> transArray;
-  aDataTransfer->GetTransferables(getter_AddRefs(transArray), dragTarget);
+  nsCOMPtr<nsISupportsArray> transArray = aDataTransfer->GetTransferables(dragTarget);
   if (!transArray)
     return false;
 
   // XXXndeakin don't really want to create a new drag DOM event
   // here, but we need something to pass to the InvokeDragSession
   // methods.
   nsCOMPtr<nsIDOMEvent> domEvent;
   NS_NewDOMDragEvent(getter_AddRefs(domEvent), aPresContext, aDragEvent);

Comment 76

[Neil Deakin]

Thanks! By coincidence I had updated the patch yesterday as well, but your version is fine too. It looks like you generated it by ignoring whitespace though. This makes it harder to read when reviewing so generally it should only be done if the reviewer requests it.

> 
> I didn't do this one. @Neil, have you done this already/could you please do
> this?

Another bug should be filed for this.


> > Hmm, this seems wrong.  I think you're missing a return statement.
> 
> I've just placed a "return NS_OK" but have no clue if that's the right
> things to do there. Please double check!

return NS_OK is correct here.


> Can you please give me a pointer on how to implement this? I guess it should
> be done using an enum, but then I don't know how to make it be on the
> |nsDOMDataTransfer| directly. Should the enum be named something like
> |nsDOMDataTransferType| instead?

I think this is a silly request so I just ignored it.


I never finished this patch as I reviewed in more details compared to the spec at http://www.w3.org/TR/clipboard-apis/ and for compatibility with other browsers.

For part 2 of the patches, I think I'm going to just add a clipboardData property to nsIClipboard instead for now.

Comment 77

[Neil Deakin]

I don't think we need another round of review here. I did some testing today and there are a only minor differences between this patch and Chrome/Webkit, all of which could be handled in followup bugs. Julian, could you update the patch, and run it on a tryserver if you haven't already and then we can check this in. Maybe Ehsan can comment if he thinks there are any issues here.

- If you cache the clipboardData object and access it outside of the event path, Chrome clears it and makes changes have no effect. We could just make it readonly.
- oncut="event.preventDefault()" prevents cutting in our patch. On Chrome, this causes an oridinary cut (which is incorrect according to the spec)
- Chrome implements the DataTransferItemList stuff from the drag and drop spec but for some reason, pasted data is often duplicated, meaning two items exist, or an extra empty item exists. Since this is described by a different spec and isn't actually needed for clipboard operations as they only contain one item, this feature doesn't need to be handled here. 
- html is formatted quite differently between both browsers. The spec contains some sanitizing and formatting text that could be done in a followup bug.

Other spec differences:
- A followup bug would be to add the clipboard event constructors. Or, probably just remove them from the spec.
- the spec provides a mechanism to clear the clipboard or remove specific types. Not sure why.
- the spec wants clipboard events to fire on keydown. This section should be removed as this behaviour is platform specific. On Mac, for example, keyboard shortcuts fire on keypress, and repeat if you hold the key down.

Comment 78

[Julian Viereck]

Attachment: Rebase v3

This is the patch I wanted to push to try but there seems to be an issue with my account.

Comment 79

[Julian Viereck]

:RyanVM pushed the v3 patch to try: https://tbpl.mozilla.org/?tree=Try&rev=3214821a9727

Comment 80

[Julian Viereck]

(In reply to Julian Viereck from comment #79)
> :RyanVM pushed the v3 patch to try:
> https://tbpl.mozilla.org/?tree=Try&rev=3214821a9727

There is one test failing on Android that might be related to this patch:

1918 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_clipboard_events.html | paste text/x-moz-url multiple types - got "", expected "http://www.mozilla.org"

See:

  https://tbpl.mozilla.org/php/getParsedLog.php?id=19608455&tree=Try

Neil, any idea?

Comment 81

[Neil Deakin]

I would guess that our android clipboard code doesn't handle x-moz-url. If that's the case, we should just disable that line of the test on Android.

Comment 82

[Julian Viereck]

From IRC: http://irclog.gr/#show/irc.mozilla.org/mobile/234940

  jviereck	Does the firefox for android clipboard code handle x-moz-url? See this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=407983#c81
  mfinkle	jviereck, it does not explicitly handle it
  jviereck	mfinkle: do you think it's okay to diable the test for Android then and open a follow up?
  mfinkle	jviereck, yes

Will open a follow up bug and disable the unit test.

Comment 83

[Julian Viereck]

Attachment: Disable failing test on Android

Disable the failing test as discussed in comment #81.

Push to try: https://tbpl.mozilla.org/?tree=Try&rev=abb606d64923

Comment 84

[Julian Viereck]

(In reply to Julian Viereck from comment #83)
>
> Push to try: https://tbpl.mozilla.org/?tree=Try&rev=abb606d64923

Look greenish to me.

@Neil, can we set checkin-needed?

Comment 85

[Neil Deakin]

Comment on attachment 712483 [details] [diff] [review]
Disable failing test on Android

Yes, this looks good. Since Ehsan's away, I'm going to ask for superreview first then we should be good to check in.

Comment 86

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 712483 [details] [diff] [review]
Disable failing test on Android


>-  // next, fire the cut or copy event
>+  // next, fire the cut, copy or paste event
>+  // XXXndeakin why was a preference added here without a running-in-chrome check?
Please file a followup for the XXXndeakin

>+  bool doDefault = true;
>+  nsRefPtr<nsDOMDataTransfer> clipboardData;
>   if (Preferences::GetBool("dom.event.clipboardevents.enabled", true)) {
>+    clipboardData = new nsDOMDataTransfer(aType, aType == NS_PASTE);
>+
>     nsEventStatus status = nsEventStatus_eIgnore;
>-    nsEvent evt(true, aType);
>-    nsEventDispatcher::Dispatch(content, presShell->GetPresContext(), &evt, nullptr,
>-                                &status);
>-    // if the event was cancelled, don't do the clipboard operation
>-    if (status == nsEventStatus_eConsumeNoDefault)
>-      return false;
>+    nsClipboardEvent event(true, aType);
>+    event.clipboardData = clipboardData;
>+    presShell->HandleDOMEventWithTarget(content, &event, &status);
>+    doDefault = (status != nsEventStatus_eConsumeNoDefault);
>   }
Why s/nsEventDispatcher::Dispatch/HandleDOMEventWithTarget/ ?


>   
>-  if (presShell->IsDestroying())
>-    return false;
>-
>   // No need to do anything special during a paste. Either an event listener
>   // took care of it and cancelled the event, or the caller will handle it.
>-  // Return true to indicate the event wasn't cancelled.
>-  if (aType == NS_PASTE)
>-    return true;
>+  // Return true to indicate that the event wasn't cancelled.
>+  if (aType == NS_PASTE) {
>+    // Clear and mark the clipboardData as readonly. This prevents someone
>+    // from reading the clipboard contents after the paste event has fired.
>+    clipboardData->ClearAll();
>+    clipboardData->SetReadOnly();
>+    return doDefault;
>+  }
Hmm, how do browsers handle modal dialogs happening during paste event?

>-  if (NS_FAILED(nsCopySupport::HTMLCopy(sel, doc, nsIClipboard::kGlobalClipboard)))
>+    rv = HTMLCopy(sel, doc, nsIClipboard::kGlobalClipboard);
>+    if (NS_FAILED(rv))
>     return false;
if (expr) {
  stmt;
}


>+  }
>+  else {
else { should be in the previous line


>-  // the effect of updating the paste menu item.
>+  // the effect of updating the enabled state of the paste menu item.
>+  if (doDefault || count)
>   piWindow->UpdateCommands(NS_LITERAL_STRING("clipboard"));
if (expr) {
  stmt;
}

> 		nsDOMNotifyAudioAvailableEvent.cpp \
> 		nsDOMSimpleGestureEvent.cpp \
> 		nsDOMEventTargetHelper.cpp \
> 		nsDOMScrollAreaEvent.cpp \
> 		nsDOMTransitionEvent.cpp \
> 		nsDOMAnimationEvent.cpp \
> 		nsDOMTouchEvent.cpp \
> 		nsDOMCompositionEvent.cpp \
>+        nsDOMClipboardEvent.cpp \
Align properly


>+nsDOMClipboardEvent::nsDOMClipboardEvent(nsPresContext* aPresContext,
>+                                         nsClipboardEvent* aEvent)
>+  : nsDOMEvent(aPresContext, aEvent ? aEvent :
>+               new nsClipboardEvent(false, 0))
>+{
>+  if (aEvent) {
>+    mEventIsInternal = false;
>+  }
>+  else {
else { to the previous line


>+nsDOMClipboardEvent::~nsDOMClipboardEvent()
>+{
>+  if (mEventIsInternal) {
>+    if (mEvent->eventStructType == NS_CLIPBOARD_EVENT) {
Perhaps merge the expressions within the ifs so that there would be just
one if


>+class nsDOMClipboardEvent : public nsIDOMClipboardEvent,
>+                            public nsDOMEvent
Inherit nsDOMEvent first



>+  else if (mIsExternal) {
should be in the previous line

>+    if (aEventType == NS_PASTE) {
>+      CacheExternalClipboardFormats();
>+    }
>+    else if (aEventType >= NS_DRAGDROP_EVENT_START && aEventType <= NS_DRAGDROP_LEAVE_SYNTH) {
ditto

>+  // only the first item is valid for clipboard events
>+  if (aIndex > 0 &&
>+      (mEventType == NS_CUT || mEventType == NS_COPY || mEventType == NS_PASTE))
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
if (exptr) {
  stmt;
}

>+

>+  // only the first item is valid for clipboard events
>+  if (aIndex > 0 &&
>+      (mEventType == NS_CUT || mEventType == NS_COPY || mEventType == NS_PASTE))
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
ditto.
Also, s/only/Only/


>+  // only the first item is valid for clipboard events
>+  if (aIndex > 0 &&
>+      (mEventType == NS_CUT || mEventType == NS_COPY || mEventType == NS_PASTE))
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
ditto


>+  // only the first item is valid for clipboard events
>+  if (aIndex > 0 &&
>+      (mEventType == NS_CUT || mEventType == NS_COPY || mEventType == NS_PASTE))
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
ditto



>+nsDOMDataTransfer::GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext)
>+{
>+  nsTArray<TransferItem>& item = mItems[aIndex];
>     uint32_t count = item.Length();
>     if (!count)
>-      continue;
>+    return nullptr;
missing indentation, and while you're here,
if (expr) {
  stmt;
}


>+  bool added = false;
>     for (uint32_t f = 0; f < count; f++) {
>-      TransferItem& formatitem = item[f];
>+    const TransferItem& formatitem = item[f];
Missing indentation?

>       nsresult rv = transferable->SetTransferData(format, convertedData, length);
>       if (NS_FAILED(rv))
>-        return;
>+      return nullptr;
same here


>+  for (uint32_t f = 0; f < mozilla::ArrayLength(formats); f++) {
++f please


>+[scriptable, builtinclass, uuid(8D92944A-F2E5-41F4-9CF3-D85043B90CAC)]
>+interface nsIDOMClipboardEvent : nsIDOMEvent
>+{
>+  readonly attribute nsIDOMDataTransfer clipboardData;
>+
>+};

Per spec there should be a dictionary for ClipboardEvent

Comment 87

[Julian Viereck]

Attachment: Right patch to disable failing test on Android

The patch "712483: Disable failing test on Android" was the same as "712145: Rebase v3". This patch is the patch you're looking for to disable the failing test.  

This is the second patch together with "712145: Rebase v3" that I've pushed to try in https://tbpl.mozilla.org/?tree=Try&rev=abb606d64923.

Comment 88

[Julian Viereck]

(In reply to Olli Pettay [:smaug] from comment #86)
> Comment on attachment 712483 [details] [diff] [review]
> [...]

Neil, what's your plan on how to handle the feedback from :smaug? I can improve the patch in terms of the "missing indentions", "if (expr) { ...}" and such. Can you handle:

- Please file a followup for the XXXndeakin
- Hmm, how do browsers handle modal dialogs happening during paste event?
- Per spec there should be a dictionary for ClipboardEvent
- Why s/nsEventDispatcher::Dispatch/HandleDOMEventWithTarget/ ?

then? Or do you think it's makes more sense if you do all the fixes on your own?

Comment 89

[Neil Deakin]

Julian, if you could please update the patch as much as you can, then I can update the remaining issues, thanks.

Comment 90

[Julian Viereck]

Attachment: WIP 4 - fix nits

This patch fixes the review-style comments made by :smaug in comment #86.

Neil, I leave the open issues raised by :smaug in comment #86 to you as per comment #89.

Comment 91

[Neil Deakin]

Attachment: Add support for clipboardData in cut/copy/paste events

Comment 92

[Neil Deakin]

Attachment: Add clipboard constructor

Comment 93

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 718977 [details] [diff] [review]
Add clipboard constructor

Could you  add [noscript] void InitClipboardEvent to nsIDOMClipboardEvent interface and just call that in InitFromCtor.
That would be consistent with other events.

Comment 94

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 718975 [details] [diff] [review]
Add support for clipboardData in cut/copy/paste events

>+NS_IMETHODIMP
>+nsDOMClipboardEvent::GetClipboardData(nsIDOMDataTransfer** aClipboardData)
>+{
>+  nsClipboardEvent* event = static_cast<nsClipboardEvent*>(mEvent);
>+
>+  MOZ_ASSERT(mEventIsInternal || event->clipboardData);
>+  if (!mEventIsInternal && !event->clipboardData) {
>+    event->clipboardData =
>+      new nsDOMDataTransfer(event->message, event->message == NS_PASTE);
>+  }
I don't understand this. We assert that either event is internal or we have
clipboard data, yet we have 'if' for the case when we have non-internal event
missing clipboard data.

>+nsDOMDataTransfer::GetTransferable(uint32_t aIndex, nsILoadContext* aLoadContext)
>+{
>+  nsTArray<TransferItem>& item = mItems[aIndex];
This is tiny bit error prone. Could you add a check that aIndex valid
>+    if (supported) {
>+      if (strcmp(formats[f], kUnicodeMime) == 0) {
>+        SetDataWithPrincipal(NS_LITERAL_STRING("text/plain"), nullptr, 0, nullptr);
>+      }
>+      else {
if (expr) {
  stmt;
} else {
  stmt2;
}


>+  if (mEventType == NS_PASTE) {
>+    MOZ_ASSERT(aIndex == 0, "index in clipboard must be 0");
>+
>+    nsCOMPtr<nsIClipboard> clipboard = do_GetService("@mozilla.org/widget/clipboard;1");
>+    if (!clipboard) {
>+      return;
>+    }
>+
>+    clipboard->GetData(trans, nsIClipboard::kGlobalClipboard);
>+  }
>+  else {
ditto

>+  // converts the data for a single item at aIndex into an nsITransferable object.
>+  already_AddRefed<nsITransferable>  GetTransferable(uint32_t aIndex,
>+                            nsILoadContext* aLoadContext);
align parameters

Comment 95

[Neil Deakin]

Attachment: Part 1 - Add support for clipboardData in cut/copy/paste events

Comment 96

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 720140 [details] [diff] [review]
Part 1 -  Add support for clipboardData in cut/copy/paste events


>+    // if the format is supported, add an item to the array with null as
>+    // the data. When retrieved, GetRealData will read the data.
>+    if (supported) {
>+      if (strcmp(formats[f], kUnicodeMime) == 0) {
>+        SetDataWithPrincipal(NS_LITERAL_STRING("text/plain"), nullptr, 0, nullptr);
>+      } else {
>+        if (strcmp(formats[f], kURLDataMime) == 0)
>+          SetDataWithPrincipal(NS_LITERAL_STRING("text/uri-list"), nullptr, 0, nullptr);
>+        SetDataWithPrincipal(NS_ConvertUTF8toUTF16(formats[f]), nullptr, 0, nullptr);
>+      }
I really wish we wouldn't almost copy-paste this code from nsDOMDataTransfer::CacheExternalFormats()
Also, why null principal?

Comment 97

[Piyush Agarwal]

@Neil Deakin - Is this patch being considered for the ESR and Latest Stable releases? If so is there an ETA for adding this support?

Thanks!
Piyush

Comment 98

[Olli Pettay [:smaug][[e-mail]]]

This is certainly a feature which shouldn't go to stable ESR17, nor FF19/20/21.
Needs normal testing.

Comment 99

[Piyush Agarwal]

@Olli Pettay, I understand. Unfortunately we have a use case where we have users pasting text into a div that is editable, but for the paste content we would like the paste to go through as plain text rather than keeping the full HTML styles etc.. (Which I can do via clipboard in the other browsers.) Unfortunately I have been unable to figure out what code a textarea runs when rich/html text is pasted in it to format it, so I cannot reproduce the same formatting when trying to do it after the default paste goes through. Any thoughts? (Or I can open a new topic somewhere if that should be discussed elsewhere).

Comment 100

[Neil Deakin]

Attachment: Part 1 - Add support for clipboardData in cut/copy/paste events

Comment 101

[Neil Deakin]

Attachment: Part 2 - add clipboard constructor

Comment 102

[(no longer active)]

(In reply to comment #99)
> @Olli Pettay, I understand. Unfortunately we have a use case where we have
> users pasting text into a div that is editable, but for the paste content we
> would like the paste to go through as plain text rather than keeping the full
> HTML styles etc.. (Which I can do via clipboard in the other browsers.)
> Unfortunately I have been unable to figure out what code a textarea runs when
> rich/html text is pasted in it to format it, so I cannot reproduce the same
> formatting when trying to do it after the default paste goes through. Any
> thoughts? (Or I can open a new topic somewhere if that should be discussed
> elsewhere).

A common workaround for doing this kind of thing before this bug is fixed to put focus into a textarea immediately when pasting to make sure that the clipboard contents are pasted into the textarea, and then extract the value of the textarea and use it inside your contentEditable element (see <http://stackoverflow.com/questions/15125217/convert-html-to-plain-text-in-contenteditable> for example).  This sucks but unfortunately there is no better workaround that I know of, and this fix is definitely not something that we can take on stable branches...

Comment 103

[Piyush Agarwal]

(In reply to :Ehsan Akhgari from comment #102)
> A common workaround for doing this kind of thing before this bug is fixed to
> put focus into a textarea immediately when pasting to make sure that the
> clipboard contents are pasted into the textarea, and then extract the value
> of the textarea and use it inside your contentEditable element (see
> <http://stackoverflow.com/questions/15125217/convert-html-to-plain-text-in-
> contenteditable> for example).  This sucks but unfortunately there is no
> better workaround that I know of, and this fix is definitely not something
> that we can take on stable branches...

Hello Ehsan, yep we actually do that for the keyboard ctrl-v/shift-ins paste event. But we need this functionality to work for the right click and edit-paste menu options as well. If I try to do the focus redirection in the actual paste event (which is the only event I see from the menu pastes), then the redirect never happens. The paste still occurs on the original div.

Comment 104

[(no longer active)]

(In reply to comment #103)
> (In reply to :Ehsan Akhgari from comment #102)
> > A common workaround for doing this kind of thing before this bug is fixed to
> > put focus into a textarea immediately when pasting to make sure that the
> > clipboard contents are pasted into the textarea, and then extract the value
> > of the textarea and use it inside your contentEditable element (see
> > <http://stackoverflow.com/questions/15125217/convert-html-to-plain-text-in-
> > contenteditable> for example).  This sucks but unfortunately there is no
> > better workaround that I know of, and this fix is definitely not something
> > that we can take on stable branches...
> 
> Hello Ehsan, yep we actually do that for the keyboard ctrl-v/shift-ins paste
> event. But we need this functionality to work for the right click and
> edit-paste menu options as well. If I try to do the focus redirection in the
> actual paste event (which is the only event I see from the menu pastes), then
> the redirect never happens. The paste still occurs on the original div.

Unfortunately I don't know of a work-around for that.

Comment 105

[Neil Deakin]

Checked into inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cc899b40731
https://hg.mozilla.org/integration/mozilla-inbound/rev/b123c8210d41

Comment 106

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1cc899b40731
https://hg.mozilla.org/mozilla-central/rev/b123c8210d41

Comment 107

[Alex Keybl [:akeybl]]

This bug has been listed as part of the Aurora 22 release notes in either [1], [2], or both. If you find any of the information or wording incorrect in the notes, please email [e-mail] asap. Thanks!

[1] https://www.mozilla.org/en-US/firefox/22.0a2/auroranotes/
[2] https://www.mozilla.org/en-US/mobile/22.0a2/auroranotes/

Comment 108

[Steven Saviano]

Quick question: I am looking into using this API in Google Docs.

It does not appear this supports writing a custom mimetype to the clipboardData object. Is that true?

For instance, on copy:
e.clipboardData.setData('text/custom-mimetype',  'data');

The on paste that data is not available. Is this expected? Are custom mimetypes planned to be supported similar to Chrome? If it should work, shall I open a bug?

Thanks!

Comment 109

[Neil Deakin]

Only text, urls, html and files are currently supported for pasting. Other types may be supported to be copied to the clipboard, but these are platform specific.

Feel free to file a bug on support for other types of data. There's lots of work to be done to improve the situation for clipboard and drag/drop so that there is more consistency between platforms.

Comment 110

[Steven Saviano]

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=860857

Comment 111

[Alfonso Martinez]

Filed Bug 864052

Comment 112

[Spocke]

I think this https://bugzilla.mozilla.org/show_bug.cgi?id=850663 bug report should be blocking for this feature. Since the clipboard data would be pretty useless from a rich text editor standpoint if contents can't be extracted when pasting from MS Office etc.

Comment 113

[Jesse Ruderman]

Spocke, are you saying we should disable this feature until bug 850663 is fixed?  Or just that you'd like us to prioritize bug 850663?

Comment 114

[Neil Deakin]

Bug 850663 is just a bug in this feature, and not a regression, and doesn't need to block this bug. For whatever reason, html data from word isn't being reflected in the clipboardData object.

Comment 115

[Spocke]

I think solving pasting from Word is a very important issue since we had to add browser sniffs for Gecko in our feature detect for ClipboardEvent and disable it. Since we would otherwise get bug reports on existing logic that they can no longer paste rich content from word.

Comment 116

[Vincent Lefevre]

I suspect that this enhancement introduced a new bug under Linux, which has security implications. See bug 894736 (bug 894736 comment 7 for the test with mozregression).

Comment 117

[Jean-Yves Perrier [:teoli]]

Documentation updated:
https://developer.mozilla.org/en-US/docs/Web/API/ClipboardEvent.clipboardData
https://developer.mozilla.org/en-US/docs/Web/API/ClipboardEvent
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/copy
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/cut
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/paste

(Thanks to Jesper Kristensen who did most of it)