714115 - CF_HTML Bug when pasting from Word and Excel if you use a , instead of . for your decimal point

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Loic]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

STR:

1) Open a .doc file containing a text with styles, links, notes, etc. with MS Office Word or MS Word Viewer (see http://support.microsoft.com/kb/891090).
NB: you can use the testcase (.doc file) attached to the bug report.
2) Select all the text (Ctrl+A) and copy (Ctrl+C).
3) Paste (Ctrl+V) the text into HTML editors like http://www.tinymce.com/tryit/full.php or http://ckeditor.com/demo


Actual results:

Only the text is copied, all elements like styles, links, notes, etc are removed. HTML structure is rebuilded too.


Expected results:

Elements should be preserved after copying the text from MS Word to HTML editors in Firefox.
It works in FF8 (and previous versions) but not in FF9 (and later like FF12).

Comment 1

[Loic]

Attachment: Testcase .doc file

Comment 2

[Josh Matthews [:jdm]]

Thanks for the report, Loic! Could you narrow down when this regressed by using the mozregression tool (http://harthur.github.com/mozregression/)?

Comment 3

[Loic]

Okay, I'm going to try, that will be my first regression narrowing. :)

Comment 4

[Loic]

So, my 1st result of narrowing (that was fun):

--repo=mozilla-central
Last good nightly: 2011-09-21
First bad nightly: 2011-09-22
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8bd19f6abbb&tochange=4495e1f795c2

--repo=mozilla-inbound
Last good nightly: 2011-09-21
First bad nightly: 2011-09-22
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=069e927983a9&tochange=cf4a13b84474

I'm not completely sure but I would say the regression is in:
Bug 598289 - Unable to paste CF_HTML from clipboard if StartHTML/EndHTML set to -1

Comment 5

[Josh Matthews [:jdm]]

Great, thank you for investigating! I agree with your assessment, and I've kicked off a build with the patches from bug 598289 backed out. You should be able to download it from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-92323f1901d2 in a couple hours, and then we can confirm our suspicions.

Comment 6

[Loic]

Josh, I tested with your try-build (92323f1901d2) and I can confirm the regression is gone. In both HTML editors, the text copied from MS Word keeps its styles, notes, etc.

Comment 7

[Josh Matthews [:jdm]]

Brian, looks like the ball's in your court now.

Comment 8

[Brian R. Bondy [:bbondy]]

Just wanted to mention with Word 2010 this works correctly even in FF9-FF12, but I'll investigate further by using Word Viewer.

Comment 9

[Loic]

Yes, I forgot to mention I tried with Word 2003 (in my STR). And I think Word Viewer is based on Word 2003 kernel.

Comment 10

[Loic]

Some users reported copying fails from LibreOffice 3.4.4 Final too.
Cf http://www.libreoffice.org/download/

Comment 11

[mohammed.habsaoui]

hello,
i'm using ms word 2010 with ff 9.0.1 but it still doesn't work.
Is there any solution to this issue ?
thx

Comment 12

[Brian R. Bondy [:bbondy]]

I plan to look into this task this week.

> i'm using ms word 2010 with ff 9.0.1 but it still doesn't work.
> Is there any solution to this issue ?

Word 2010 shouldn't be affected, maybe what your issue is, is unrelated to this task?  Can you try with Firefox 8 or lower?

Comment 13

[mohammed.habsaoui]

works with ff8 but that's not the best solution =p

Comment 14

[Brian R. Bondy [:bbondy]]

Thanks for the info.  
Sorry I just wanted more information, wasn't proposing it as a workaround or solution :)

Comment 15

[Brian R. Bondy [:bbondy]]

Still could not reproduce with Nightly nor Aurora by using Word Viewer 2003.

1. Downloaded and installed http://www.microsoft.com/download/en/details.aspx?id=4
2. Opened your attachment .doc file.
3. Ctrl + A, Ctrl + C in the .doc file.
4. Ctrl + V in either of the 2 sites you referenced.

All formatting stays intact. 
I also tried copying just a portion of the document and that also works.

I suspect I'm missing a step? I'm on Windows 7 x64 by the way.

Comment 16

[Loic]

Nope, your STR are the same as mine. I have just retested with the latest Aurora & Nightly, and I'm able to reproduce the bug, it's weird.

Snapshot of what I see after pasting the text:
+ ckeditor.com: http://i.imgur.com/Yj7Aw.jpg
+ tinymce.com: http://i.imgur.com/pI53w.jpg

I'm on the same OS as you. Are you running Word Viewer 2003 in a special compatibility mode? Mine is WORDVIEW.EXE *32.

Comment 17

[Brian R. Bondy [:bbondy]]

Attachment: ClipSpy

Ya on my screen it keeps the formatting (same as FF8 for me).  The process name is the same for me. I am just using the default just installed mode for Word viewer. 

I have an idea, could you please download this:
http://www.codeproject.com/KB/clipboard/clipspy.aspx
In the download package there is a .exe in the bin folder.
(I attached just ClipSpy.exe to this ticket)

1. Start ClipSpy.exe
2. copy from word Viewer in the same way you have been. 
3. Click on HTML Format in ClipSpy
4. Click on File Save selected data, call it out.txt
5. Attach that file to this ticket, also a screenshot of ClipSpy would help too.

I have a feeling that your HTML Format will be different from mine, or maybe you won't have one at all.

Comment 18

[Brian R. Bondy [:bbondy]]

do your decimal points display as , or .?

Comment 19

[Loic]

Attachment: testcase.doc_HTML.format_outpout

Comment 20

[Loic]

Attachment: testcase.doc_HTML.format_snapshot

Comment 21

[Loic]

(In reply to Brian R. Bondy [:bbondy] from comment #17)

> 5. Attach that file to this ticket, also a screenshot of ClipSpy would help
> too.
Done.

(In reply to Brian R. Bondy [:bbondy] from comment #18)
> do your decimal points display as , or .?
Where? Screen? OS? Applications?
I'm using a French OS, so it's essentially with ",".

Comment 22

[Brian R. Bondy [:bbondy]]

Awesome thanks for the info.

Control panel | Region and Language | Formats | Customize Format | Decimal Symbol:
If I change from . to , I can reproduce.
If I change from , to . I cannot reproduce.
Note: You have to restart Firefox in between changes to see the effect, but you can keep the same clipboard content from Word.


Fix shouldn't be hard.

Comment 24

[Brian R. Bondy [:bbondy]]

The problem seems to be from this line:

+  int numFound = sscanf((char*)*outData, "Version:%f\nStartHTML:%d\nEndHTML:%d", 
+                         &vers, &startOfData, &endOfData);

The CF_HTML format has a period in its version (as per the spec), but this call will fail with %f if your local settings are set to , for the decimal point.  sscanf with %f I think will only work with comma decimal points if that's your locale settings on Windows.

Comment 25

[Loic]

You're right, changing the decimal point from "," to "." fixes the bug. Good catch! :)

Comment 26

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Tested and it works with any type of decimal point.
After this review I'll request Aurora and Beta approval for this patch.

Comment 27

[Loic]

I tested with various characters/strings as decimal point.

Windows defines a max 3-digit value as decimal point.
If the 1st digit begings by "." (".XY" where X & Y are random chars), it works, but it doesn't if the 1st digit is different from "." (like ":" or "b8").

Comment 28

[Brian R. Bondy [:bbondy]]

Thanks, I'm just reading it in as a string since we don't actually use the value that was causing problems.

Comment 29

[Brian R. Bondy [:bbondy]]

I tested the format string on both Windows VS10 and g++ on Ubuntu by the way.

Comment 30

[Brian R. Bondy [:bbondy]]

Pushed to try, will push to inbound once that passes.  Thanks for the quick review.

Comment 32

[Brian R. Bondy [:bbondy]]

Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9fdf008925ce

Comment 33

[Brian R. Bondy [:bbondy]]

Comment on attachment 588256 [details] [diff] [review]
Patch v1.

[Approval Request Comment]
Regression caused by (bug #): bug 598289 
User impact if declined: HTML format pasting will not work, instead will be another format like text or image that is available in clipboard.
Testing completed (on m-c, etc.): local testing, pushed to m-i, will be on m-c soon.
Risk to taking this patch (and alternatives if risky): low

This is a regression introduced in Mozilla 9.  The HTML format is very important in Windows and won't be used as of Mozilla 9 code if you have a comma instead of a period as your decimal point in Windows settings.  I would like to see this fixed on both beta and aurora as the risk is low.

Comment 34

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/9fdf008925ce

Comment 36

[Alex Keybl [:akeybl]]

(In reply to Brian R. Bondy [:bbondy] from comment #33)
> [Approval Request Comment]
> Regression caused by (bug #): bug 598289 
> Risk to taking this patch (and alternatives if risky): low

Hi Brian - given where we are in the beta cycle, we have to exercise caution. Do you believe this patch is on par with the risk of backing out bug 598289, or that keeping 598289 in FF10 has significant reward?

I'm currently leaning towards taking this patch on Aurora and backing out 598289 for FF10.

Comment 37

[Alex Keybl [:akeybl]]

Comment on attachment 588256 [details] [diff] [review]
Patch v1.

[Triage Comment]
Although we're still considering whether or not to take this for beta, there's no need to block uplifting to aurora.

Comment 38

[Brian R. Bondy [:bbondy]]

> or that keeping 598289 in FF10 has significant reward?

I don't think the reward is significant. I do believe 598289 does provide benefit but not enough to justify any amount of last minute risk.

> I'm currently leaning towards taking this patch on Aurora and backing out 
> 598289 for FF10.

That sounds good, the 3 patches from bug 598289 would have to be backed out from FF10.  

Please advise, I can do as you say tonight for Aurora and Beta.

Comment 39

[Brian R. Bondy [:bbondy]]

I don't think the reward is significant to have either a backout or this patch applied though to Beta.

Comment 40

[Brian R. Bondy [:bbondy]]

sorry Comment 39 should read:
I DO think the reward is significant to have either a backout, or this patch applied to Beta.

Comment 42

[Brian R. Bondy [:bbondy]]

Pushed to aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/274a4cfbf3fd

Recommendation for beta is to backout the 3 patches from bug 598289.

Comment 44

[Alex Keybl [:akeybl]]

Comment on attachment 588256 [details] [diff] [review]
Patch v1.

[Triage Comment]
Going with Brian's recommendation to backout bug 598289. Feel free to use either bug for the backout (we're now tracking both).

Comment 45

[Brian R. Bondy [:bbondy]]

Bug 598289 was backed out of beta, I indicated the changeset in that bug.

Comment 47

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0

The elements are preserved when performing the steps from comment 0.

Comment 48

[Loic]

(In reply to Ioana Budnar [QA] from comment #47)
> The elements are preserved when performing the steps from comment 0.
Hi. Did you set the decimal point as "," (or another random char different of ".") in Windows before testing?

Comment 49

[Ioana (away)]

(In reply to Loic from comment #48)
> (In reply to Ioana Budnar [QA] from comment #47)
> > The elements are preserved when performing the steps from comment 0.
> Hi. Did you set the decimal point as "," (or another random char different
> of ".") in Windows before testing?

Yes, Loic, I verified it works well both with "." and ",".