Last Comment Bug 572642 - Paste text from OpenOffice writer to Google Docs not working
: Paste text from OpenOffice writer to Google Docs not working
Status: RESOLVED FIXED
required on branches by bug 520189 [q...
: regression
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla2.0b1
Assigned To: :Ehsan Akhgari (out sick)
:
Mentors:
Depends on: 591544 591750
Blocks: CVE-2010-2769
  Show dependency treegraph
 
Reported: 2010-06-17 02:41 PDT by Andrea
Modified: 2010-08-29 11:25 PDT (History)
8 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1+
.9+
.9-fixed
.12+
.12-fixed


Attachments
Patch (v1) (1.38 KB, patch)
2010-06-21 15:20 PDT, :Ehsan Akhgari (out sick)
no flags Details | Diff | Review
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard (5.71 KB, patch)
2010-06-21 16:40 PDT, :Ehsan Akhgari (out sick)
roc: review+
dveditz: approval1.9.2.9+
dveditz: approval1.9.1.12+
Details | Diff | Review
Part2: tests (9.27 KB, patch)
2010-06-21 20:38 PDT, :Ehsan Akhgari (out sick)
roc: review+
Details | Diff | Review

Description Andrea 2010-06-17 02:41:20 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; it-IT; rv:1.9.3a6pre) Gecko/20100616 Minefield/3.7a6pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; it-IT; rv:1.9.3a6pre) Gecko/20100616 Minefield/3.7a6pre

When I try to paste some text copied from a writer document into a Google Doc using the Ctrl-V shortcut, only two '*' appear.
When I use the same paste command accessing it from the Edit menu of the browser, I get a google message saying the browser does not permit access to the clipboard contents.

Instead, pasting a text selection copied from a simple text editor (tried notepad, Textpad) works.

So a workaround for now is to paste the text into a simple text editor and only after that paste it into google docs.
Is this perhaps related to text formatting somehow? Seems a regression, since Firefox 3.6.3 isn't affected.

Also tried with a clean profile of course.

Reproducible: Always

Steps to Reproduce:
1. Open a text google doc
2. copy some text from a document opened with OpenOffice Writer (perhaps the same happens with MS Office Word, dunno)
3. paste it in the google doc using Ctrl+V or the browser Paste command in the Edit menu
Actual Results:  
The text is not pasted, either "**" appears, or a message window opens, informing that the browser does not permit access to the clipboard contents.

Expected Results:  
The text is pasted
Comment 1 XtC4UaLL [:xtc4uall] 2010-06-17 11:39:48 PDT
fyi, the opposite way has been fixed in Bug 537414 recently.
Comment 2 Andrea 2010-06-18 02:21:15 PDT
This problem however involves only Google Docs. Pasting Writer contents to forms, forum comments, even Microsoft Word Web App, works.
Comment 3 Alice0775 White 2010-06-18 10:11:14 PDT
I can reproduce with Microsoft Office Word2003 SP3 and Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100618 Minefield/3.7a6pre ID:20100618040705

It does not happen on Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6pre) Gecko/20100617 Namoroka/3.6.6pre ID:20100617041816


Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/dcfe95c71a04
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100614 Minefield/3.7a6pre ID:20100614135921

Fails:
http://hg.mozilla.org/mozilla-central/rev/3c23833e5c1b
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100614 Minefield/3.7a6pre ID:20100614145614

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dcfe95c71a04&tochange=3c23833e5c1b

Candidate:
Mon Jun 14 13:52:39 2010 -0700	208d7f999037	Ehsan Akhgari Bug 520189 - Fix copy and test for the HTML editor
Comment 4 Alice0775 White 2010-06-18 11:17:10 PDT
This is similar to Bug 572637 - Paste image into Compose window broken with Shredder
Comment 5 XtC4UaLL [:xtc4uall] 2010-06-18 11:56:28 PDT
Anyone able to test against Linux/MacOS (using OpenOffice)?
Comment 6 :Ehsan Akhgari (out sick) 2010-06-18 12:26:44 PDT
On Mac, a much worse issue exists, which is actually an OpenOffice bug.  I've filed that as <http://www.openoffice.org/issues/show_bug.cgi?id=112525>.
Comment 7 :Ehsan Akhgari (out sick) 2010-06-18 12:29:40 PDT
I'm currently building on Windows to see what's going on.  This could be a Windows-only issue, because how HTML is handled in the clipboard is totally different in Windows and other OSes.
Comment 8 :Ehsan Akhgari (out sick) 2010-06-18 12:30:38 PDT
(In reply to comment #4)
> This is similar to Bug 572637 - Paste image into Compose window broken with
> Shredder

Not really!  :-)
Comment 9 Alice0775 White 2010-06-18 12:46:07 PDT
This problem happens on rich text editor of  http://www.mozilla.org/editor/midasdemo/  too.

(In reply to comment #5)
> Anyone able to test against Linux/MacOS (using OpenOffice)?

I can *Not* reproduced on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100618 Minefield/3.7a6pre + Openoffice 2.4.1


(In reply to comment #8)
> (In reply to comment #4)
> > This is similar to Bug 572637 - Paste image into Compose window broken with
> > Shredder
> 
> Not really!  :-)

Oh my bad.
Comment 10 :Ehsan Akhgari (out sick) 2010-06-18 12:48:47 PDT
(In reply to comment #9)
> This problem happens on rich text editor of 
> http://www.mozilla.org/editor/midasdemo/  too.

Yes.  It happens on any contenteditable element.

> (In reply to comment #5)
> > Anyone able to test against Linux/MacOS (using OpenOffice)?
> 
> I can *Not* reproduced on Mozilla/5.0 (X11; U; Linux i686; en-US;
> rv:1.9.3a6pre) Gecko/20100618 Minefield/3.7a6pre + Openoffice 2.4.1

This confirms my suspicion that this has something to do with the CF_HTML clipboard type.  Thanks for testing this!
Comment 11 :Ehsan Akhgari (out sick) 2010-06-21 15:20:53 PDT
Created attachment 452868 [details] [diff] [review]
Patch (v1)

We don't need to allow styles while parsing the context.  This fixes pasting from OpenOffice, and we don't want that anyway, I guess.
Comment 12 :Ehsan Akhgari (out sick) 2010-06-21 16:40:59 PDT
Created attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

Actually, there was more here than caught my eye.

What happens is that we use a comment when parsing the context for CF_HTML format from the clipboard.  But the paranoid content sink does not allow comments.  So we need to allow comments when parsing styles.  This patch does that.

I'm trying to come up with a test case for this as well.
Comment 13 :Ehsan Akhgari (out sick) 2010-06-21 20:38:57 PDT
Created attachment 452957 [details] [diff] [review]
Part2: tests

Here is a set of tests for our CF_HTML handling.  The cfhtml-*.txt files come from the Clipboard Spy tool recording the actual clipboard contents on Windows from Firefox, Chromium, OpenOffice.org and IE.
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-21 20:51:16 PDT
Comment on attachment 452957 [details] [diff] [review]
Part2: tests

lovely
Comment 15 :Ehsan Akhgari (out sick) 2010-06-22 12:12:33 PDT
(In reply to comment #14)
> (From update of attachment 452957 [details] [diff] [review])
> lovely

So, I noticed a problem with the tests on the try server.  The tests failed in nsITransferable::GetTransferData on Windows and Linux, and passed on Mac.  After debugging, I can confirm that when trying to copy the data to the clipboard, the platform APIs pretend that the data is available on the clipboard, but it really isn't.  This might explain why these tests pass on Mac, because Mac has no notion of "native html" for clipboard at all.

Given the fact that our CF_HTML code is cross-platform, and that I really don't have any idea how I can put the correct data on clipboard on Windows and Linux, I was planning to make this test Mac-only and land it.  Roc, do you agree?
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-22 12:14:14 PDT
(In reply to comment #15)
> So, I noticed a problem with the tests on the try server.  The tests failed in
> nsITransferable::GetTransferData on Windows and Linux, and passed on Mac. 
> After debugging, I can confirm that when trying to copy the data to the
> clipboard, the platform APIs pretend that the data is available on the
> clipboard, but it really isn't.  This might explain why these tests pass on
> Mac, because Mac has no notion of "native html" for clipboard at all.

I don't understand.
Comment 17 :Ehsan Akhgari (out sick) 2010-06-22 14:21:15 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > So, I noticed a problem with the tests on the try server.  The tests failed in
> > nsITransferable::GetTransferData on Windows and Linux, and passed on Mac. 
> > After debugging, I can confirm that when trying to copy the data to the
> > clipboard, the platform APIs pretend that the data is available on the
> > clipboard, but it really isn't.  This might explain why these tests pass on
> > Mac, because Mac has no notion of "native html" for clipboard at all.
> 
> I don't understand.

When I call nsIClipboard::SetData, the call returns successfully.  When I call nsIClipboard::GetData later on, that call also returns successfully.  But when I call nsITransferable::GetTransferData on the transferable passed to nsIClipboard::GetData, it throws.  Things work correctly for any mime type except application/x-moz-nativehtml, which is why I suspect that Windows and Linux do something special with CF_HTML data under the hoods...
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-22 14:32:57 PDT
Hmm, is it possible to track down the cause of the throw?
Comment 19 :Ehsan Akhgari (out sick) 2010-06-22 14:53:41 PDT
(In reply to comment #18)
> Hmm, is it possible to track down the cause of the throw?

The cause is that the platform clipboard APIs do not give us any data.  :-)

I'm not sure how we should fix that.  I guess this has something to do with the way that we should put native html data on the clipboard, but I wasn't able to figure out how to do that correctly so that we can actually get the data back from the clipboard.

Come to think of it, one other way to handle things would be for me to override the clipboard xpcom component in the test and make it return the data that I want, but I don't think that's going to catch all the ways we can fail in the future with a real clipboard.
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-22 15:10:24 PDT
Is it just a delay issue, or does the data never arrive?

If it's just a delay, we could poll in the test.
Comment 21 :Ehsan Akhgari (out sick) 2010-06-22 15:11:20 PDT
(In reply to comment #20)
> Is it just a delay issue, or does the data never arrive?
> 
> If it's just a delay, we could poll in the test.

I already poll in the test, but no, the data never arrives!
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-22 17:27:54 PDT
So does copy and paste of HTML from one Firefox instance to another work at all?
Comment 23 :Ehsan Akhgari (out sick) 2010-06-22 21:27:57 PDT
(In reply to comment #22)
> So does copy and paste of HTML from one Firefox instance to another work at
> all?

Yes, but we don't use the CF_HTML format when copying from Firefox.  We just put the HTML itself plus the context and info information as separate flavors on the clipboard <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCopySupport.cpp#204>.
Comment 24 :Ehsan Akhgari (out sick) 2010-06-22 21:28:41 PDT
In other words, we don't currently have any call site in our tree which attempts to put CF_HTML content on the clipboard.  We only support pasting it from the clipboard.
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-06-22 22:07:45 PDT
Ok then, go ahead and make the test Mac-only :-(
Comment 26 :Ehsan Akhgari (out sick) 2010-06-23 08:52:48 PDT
(In reply to comment #25)
> Ok then, go ahead and make the test Mac-only :-(

OK.  I filed bug 574005 so that I have something to stay on my conscious in the future, in case I can think of a way to fix this in the future.
Comment 28 :Ehsan Akhgari (out sick) 2010-06-23 12:12:11 PDT
Comment on attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

We have to take this patch on branches as well if we want to take the patch to bug 520189, since this is a pretty important regression from that patch.
Comment 29 Reed Loden [:reed] (use needinfo?) 2010-06-23 12:25:25 PDT
Comment on attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

> #define NS_I_PARANOID_FRAGMENT_CONTENT_SINK_IID \
>-  { 0x59ec77f5, 0x9e9b, 0x4040, \
>+  { 0x69ec77f5, 0x9e9b, 0x4040, \
>     { 0xbd, 0xe7, 0x4e, 0xd0, 0x13, 0xa6, 0x21, 0x74 } }

Please don't just change one bit in order to rev an IID. If you're going to rev it, actually rev it. This should be fixed, imho. What stops somebody in the future from doing the same thing but changing the 6 back to a 5? Better to just change it completely.
Comment 30 Benjamin Smedberg [:bsmedberg] 2010-06-23 12:38:19 PDT
Yes, the risk of collisions goes up dramatically when you don't generate a new UUID using standard tools. Please use guidgen/uuidgen to generate new UUIDs.
Comment 31 :Ehsan Akhgari (out sick) 2010-06-23 12:53:02 PDT
You're completely right, sorry about that mistake.  Fixed in <http://hg.mozilla.org/mozilla-central/rev/c8df4a1c8860>
Comment 32 Daniel Veditz [:dveditz] 2010-07-16 11:35:06 PDT
Comment on attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

> #define NS_I_PARANOID_FRAGMENT_CONTENT_SINK_IID \
>-  { 0x59ec77f5, 0x9e9b, 0x4040, \
>+  { 0x69ec77f5, 0x9e9b, 0x4040, \
>     { 0xbd, 0xe7, 0x4e, 0xd0, 0x13, 0xa6, 0x21, 0x74 } }

Is this an interface we can change on the branch?
Comment 33 :Ehsan Akhgari (out sick) 2010-07-16 16:11:49 PDT
(In reply to comment #32)
> Comment on attachment 452893 [details] [diff] [review]
> Part 1: Allow comments and disallow styles when parsing the context for CF_HTML
> content on the clipboard
> 
> > #define NS_I_PARANOID_FRAGMENT_CONTENT_SINK_IID \
> >-  { 0x59ec77f5, 0x9e9b, 0x4040, \
> >+  { 0x69ec77f5, 0x9e9b, 0x4040, \
> >     { 0xbd, 0xe7, 0x4e, 0xd0, 0x13, 0xa6, 0x21, 0x74 } }
> 
> Is this an interface we can change on the branch?

Yes.  It was introduced in bug 520189, which means that it's a new interface when/if that bug lands on branch.
Comment 34 Daniel Veditz [:dveditz] 2010-07-23 11:04:43 PDT
Comment on attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

I'm not sure we can approve this patch for the stable branches because you're changing interfaces. This one's not an .idl so maybe it's OK?
Comment 35 :Ehsan Akhgari (out sick) 2010-07-23 11:51:10 PDT
(In reply to comment #34)
> I'm not sure we can approve this patch for the stable branches because you're
> changing interfaces. This one's not an .idl so maybe it's OK?

It's OK, not because it's not an IDL interface, but because this is a new interface introduced in bug 520189.
Comment 36 :Ehsan Akhgari (out sick) 2010-07-23 11:51:30 PDT
dveditz: see comment 35 please.
Comment 37 Daniel Veditz [:dveditz] 2010-07-23 13:03:45 PDT
Comment on attachment 452893 [details] [diff] [review]
Part 1: Allow comments and disallow styles when parsing the context for CF_HTML content on the clipboard

Approved for 1.9.2.9 and 1.9.1.12, a=dveditz
Comment 39 :Ehsan Akhgari (out sick) 2010-07-23 14:20:44 PDT
Backed out because bug 520189 was backed out (see bug 520189 comment 83).
Comment 41 :Ehsan Akhgari (out sick) 2010-08-03 08:19:37 PDT
I forgot to update the status flags for branches.
Comment 42 Al Billings [:abillings] 2010-08-12 18:02:53 PDT
I cannot reproduce the original problem.

Using Openoffice 3.2.1, I've created a new document. I've done both simple text in it and text with font face, style, and size settings added.

I select the text and use ctrl-c to copy it.

I open Firefox 3.6.8, go to Google Docs, and create a new document. In that document, I use ctrl-v or the browser paste option under the edit menu. In both instances, the text appears without issues.

This was on Windows XP. 

In no case, is the pasted text replaced with **.

The a message window opening and stating that the browser does not permit access to the clipboard contents only happens if the Google Docs paste menu is used. This is a Google docs issue (and occurs in the current nightly 1.9.2.9pre build as well). 

I see no difference in behavior between 3.6.8 and the nightly 1.9.2 build and I cannot reproduce the bug.
Comment 43 :Ehsan Akhgari (out sick) 2010-08-13 12:35:46 PDT
Have you tried to see if you can reproduce the problem on a trunk version before this bug fixed?
Comment 44 Al Billings [:abillings] 2010-08-19 17:35:00 PDT
(In reply to comment #43)
> Have you tried to see if you can reproduce the problem on a trunk version
> before this bug fixed?

Yes, if I use a trunk build before the fix, the clipboard contents fail to paste. After the fix, they work.

In 1.9.2, the paste works without any problems both pre and post-fix.
Comment 45 :Ehsan Akhgari (out sick) 2010-08-22 13:36:06 PDT
(In reply to comment #44)
> (In reply to comment #43)
> > Have you tried to see if you can reproduce the problem on a trunk version
> > before this bug fixed?
> 
> Yes, if I use a trunk build before the fix, the clipboard contents fail to
> paste. After the fix, they work.
> 
> In 1.9.2, the paste works without any problems both pre and post-fix.

Hmm, this shouldn't really be happening.  I don't know why it does.  But at any rate, we do want this fix on 1.9.2...

Note You need to log in before you can comment on or make changes to this bug.