As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 598289 - Unable to paste CF_HTML from clipboard if StartHTML/EndHTML set to -1
: Unable to paste CF_HTML from clipboard if StartHTML/EndHTML set to -1
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: unspecified
: x86 Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
: Jim Mathies [:jimm]
Mentors:
Depends on: 574005 714115
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-21 03:35 PDT by ephox
Modified: 2012-01-23 14:21 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Test case with Java application and html page (2.12 KB, application/zip)
2010-09-21 03:44 PDT, ephox
no flags Details
Patch for CF_HTML handling when whole context is specified with -1 (4.75 KB, patch)
2011-08-13 14:04 PDT, Brian R. Bondy [:bbondy]
ehsan: review+
Details | Diff | Splinter Review
Patch for parsing CF_HTML in win32 widget code (3.04 KB, patch)
2011-08-13 14:06 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Patch for parsing CF_HTML in win32 widget code v2 (3.06 KB, patch)
2011-08-14 21:05 PDT, Brian R. Bondy [:bbondy]
roc: review+
Details | Diff | Splinter Review
Patch for CF_HTML handling when whole context is specified with -1 v2 (4.77 KB, patch)
2011-08-15 10:13 PDT, Brian R. Bondy [:bbondy]
ehsan: review+
Details | Diff | Splinter Review
Test case for CF_HTML parsing with startHTML and EndHTML set to -1 (2.21 KB, patch)
2011-08-17 13:42 PDT, Brian R. Bondy [:bbondy]
ehsan: review+
Details | Diff | Splinter Review

Description User image ephox 2010-09-21 03:35:27 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; en-gb) AppleWebKit/533.18.1 (KHTML, like Gecko) Version/5.0.2 Safari/533.18.5
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

If the HTML format on the clipboard has the StartHTML and EndHTML fields set to -1 (to indicate "no context" as per http://msdn.microsoft.com/en-us/library/aa767917(VS.85).aspx) pasting into a contenteditable section (or designMode iframe) fails with no output or error messages.

This is a significant problem because Java always sets StartHTML and EndHTML to -1 when copying HTML to the clipboard (in a low-level, sealed class).  As such, it's impossible to copy HTML from a Java application and paste it into Firefox on Windows.

Reproducible: Always

Steps to Reproduce:
1. Copy text from a Java JTextPane which has it's contentType set to text/html
2. Attempt to paste into a contenteditable div or editor such as TinyMCE within Firefox.
Actual Results:  
Nothing happens.

Expected Results:  
HTML content from the clipboard should have been pasted.
Comment 1 User image ephox 2010-09-21 03:44:30 PDT
Created attachment 477091 [details]
Test case with Java application and html page
Comment 2 User image Brian R. Bondy [:bbondy] 2011-08-13 14:04:15 PDT
Created attachment 552901 [details] [diff] [review]
Patch for CF_HTML handling when whole context is specified with -1

Fixed, added support for paste operations from clipboard data that include the whole context.

I will be submitting a third patch (second patch is a fix in win32 widget) for a mochitest for review by you as well separately.
Comment 3 User image Brian R. Bondy [:bbondy] 2011-08-13 14:06:32 PDT
Created attachment 552902 [details] [diff] [review]
Patch for parsing CF_HTML in win32 widget code

Clipboard operations for CF_HTML was doing bad parsing for the CF_HTML values.
Comment 4 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-14 18:08:46 PDT
I'm confused. We're parsing as %u and then checking for -1. So Windows actually sends us 2^32-1 here?
Comment 5 User image Brian R. Bondy [:bbondy] 2011-08-14 18:10:49 PDT
Yes it does. I think it would be better to do %d though so I'll submit a new patch.
Comment 6 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-14 18:13:33 PDT
Will 2^32-1 actually be parsed by %d?
Comment 7 User image Brian R. Bondy [:bbondy] 2011-08-14 18:16:59 PDT
Well I was going to use an int instead of an unsigned int.  So 2^31 would be the max (~2GB text).  Do you want me to use a 64bit int and format specifier instead?
Comment 8 User image Brian R. Bondy [:bbondy] 2011-08-14 18:17:18 PDT
2^31-1 that is
Comment 9 User image Brian R. Bondy [:bbondy] 2011-08-14 18:19:37 PDT
I could also do like the other patch and not even use sscanf and do something like FindIntegerAfterString.
Comment 10 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-14 20:36:54 PDT
I mean, is Windows sending "StartHTML:4294967295\n" or "StartHTML:-1\n"?
Comment 11 User image Brian R. Bondy [:bbondy] 2011-08-14 20:41:57 PDT
Windows always sends it as "StartHTML:-1" and never as "StartHTML:4294967295".

MSVC++ also parses both of these the same with a return value of 4294967295 inside &ui.

sscanf("-1", "%u", &ui);
sscanf("4294967295", "%u", &ui);
Both return values are also 1.

But I'm not sure if that behaviour is the same across other compilers so that's why I suggested %d.
I know that code is only hit in Windows only though so let me know how you want the modified patch.
Comment 12 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-14 20:45:43 PDT
%d into PRInt32s sounds right.
Comment 13 User image Brian R. Bondy [:bbondy] 2011-08-14 21:05:20 PDT
Created attachment 553096 [details] [diff] [review]
Patch for parsing CF_HTML in win32 widget code v2

Changed %u to %d, old code worked but it makes more sense to be %d.
Comment 14 User image :Ehsan Akhgari 2011-08-15 08:11:21 PDT
Comment on attachment 552901 [details] [diff] [review]
Patch for CF_HTML handling when whole context is specified with -1

>+  if (endHTML == -1) {
>+    const char endFragmentMarker[] = "<!--EndFragment-->";
>+    endHTML = aCfhtml.Find(endFragmentMarker);
>+    if (endHTML == -1)
>+      return PR_FALSE;
>+    endHTML += sizeof(endFragmentMarker);

Use NS_ARRAY_LENGTH please.  r=me with that.

For the test, I suggest you grab the CF-HTML data on the clipboard with Windows Clipboard Viewer or a similar tool, dump it into a file, and then just extend <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/tests/test_CF_HTML_clipboard.html?force=1> to test the new CF-HTML data.

Thanks for your patch!
Comment 15 User image Brian R. Bondy [:bbondy] 2011-08-15 10:13:03 PDT
Created attachment 553209 [details] [diff] [review]
Patch for CF_HTML handling when whole context is specified with -1 v2

Changed sizeof to NS_ARRAY_LENGTH, I also subtracted 1 from this length because I didn't mean to include the null termination char.
Comment 16 User image Brian R. Bondy [:bbondy] 2011-08-17 13:42:37 PDT
Created attachment 553892 [details] [diff] [review]
Test case for CF_HTML parsing with startHTML and EndHTML set to -1
Comment 17 User image :Ehsan Akhgari 2011-08-17 13:55:26 PDT
Comment on attachment 553892 [details] [diff] [review]
Test case for CF_HTML parsing with startHTML and EndHTML set to -1

Looks great, thanks!
Comment 18 User image Brian R. Bondy [:bbondy] 2011-09-20 12:57:59 PDT
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=9bfc89c2e2c8
Comment 21 User image Olivier Vit (just a reporter) 2011-10-17 02:11:41 PDT
Hi
Should this also fix the following scenario FF 7.0.1 release on Windows Vista 32):
Copy text containing an HTML table from a web page (Gmail) and pasting it into another web page (Google Docs) : fails silently without any content being pasted.
While pasting to MS Word, copying again from MS Word, and pasting to Google Docs works fine.
Comment 22 User image Olivier Vit (just a reporter) 2011-10-17 03:21:21 PDT
Just tested in Aurora 9.0a2 (2011-10-16) and the bug is still there, worth a bug report then ?
Comment 23 User image Brian R. Bondy [:bbondy] 2011-10-22 18:45:36 PDT
I tested with v9 and v10 and it is fixed for me.
I tested with v8 and v7 and it does not work (as the fix has not hit there yet).

Please post more detailed steps in a new bug and CC me if it still doesn't work for you.  
Note: I did find that in java 1.7 java fixed the -1 context CF_HTML so you need to use 1.6 to reproduce on the older browsers in the first place.
Comment 24 User image Brian R. Bondy [:bbondy] 2012-01-17 12:15:59 PST
> Going with Brian's recommendation to backout bug 598289. Feel free to use either  > bug for the backout (we're now tracking both).

I'll backout this task from Beta within the context of this bug. 
The changesets which will be backed out are: 

https://hg.mozilla.org/mozilla-central/rev/9e3562fefd4c
https://hg.mozilla.org/mozilla-central/rev/b7486f31f6b1
https://hg.mozilla.org/mozilla-central/rev/16c9cfa0cf84
Comment 25 User image Brian R. Bondy [:bbondy] 2012-01-17 18:54:10 PST
As described in bug 714115, I pushed the backout of the above 3 revisions to beta. They can be found aggregated in this changeset:
http://hg.mozilla.org/releases/mozilla-beta/rev/d53d327daa8d

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