Closed
Bug 598289
Opened 14 years ago
Closed 13 years ago
Unable to paste CF_HTML from clipboard if StartHTML/EndHTML set to -1
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: support, Assigned: bbondy)
References
(Depends on 1 open bug)
Details
Attachments
(4 files, 2 obsolete files)
2.12 KB,
application/zip
|
Details | |
3.06 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
4.77 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
2.21 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → netzen
Assignee | ||
Comment 2•13 years ago
|
||
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.
Attachment #552901 -
Flags: review?(ehsan)
Assignee | ||
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•13 years ago
|
||
Clipboard operations for CF_HTML was doing bad parsing for the CF_HTML values.
Attachment #552902 -
Flags: review?(roc)
I'm confused. We're parsing as %u and then checking for -1. So Windows actually sends us 2^32-1 here?
Assignee | ||
Comment 5•13 years ago
|
||
Yes it does. I think it would be better to do %d though so I'll submit a new patch.
Will 2^32-1 actually be parsed by %d?
Assignee | ||
Comment 7•13 years ago
|
||
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?
Assignee | ||
Comment 8•13 years ago
|
||
2^31-1 that is
Assignee | ||
Comment 9•13 years ago
|
||
I could also do like the other patch and not even use sscanf and do something like FindIntegerAfterString.
I mean, is Windows sending "StartHTML:4294967295\n" or "StartHTML:-1\n"?
Assignee | ||
Comment 11•13 years ago
|
||
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.
%d into PRInt32s sounds right.
Assignee | ||
Comment 13•13 years ago
|
||
Changed %u to %d, old code worked but it makes more sense to be %d.
Attachment #552902 -
Attachment is obsolete: true
Attachment #553096 -
Flags: review?(roc)
Attachment #552902 -
Flags: review?(roc)
Attachment #553096 -
Flags: review?(roc) → review+
Comment 14•13 years ago
|
||
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!
Attachment #552901 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 15•13 years ago
|
||
Changed sizeof to NS_ARRAY_LENGTH, I also subtracted 1 from this length because I didn't mean to include the null termination char.
Attachment #552901 -
Attachment is obsolete: true
Attachment #553209 -
Flags: review?(ehsan)
Updated•13 years ago
|
Attachment #553209 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 16•13 years ago
|
||
Attachment #553892 -
Flags: review?(ehsan)
Comment 17•13 years ago
|
||
Comment on attachment 553892 [details] [diff] [review] Test case for CF_HTML parsing with startHTML and EndHTML set to -1 Looks great, thanks!
Attachment #553892 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 18•13 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=9bfc89c2e2c8
Assignee | ||
Comment 19•13 years ago
|
||
Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/16c9cfa0cf84 http://hg.mozilla.org/integration/mozilla-inbound/rev/b7486f31f6b1 http://hg.mozilla.org/integration/mozilla-inbound/rev/9e3562fefd4c
Comment 20•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e3562fefd4c https://hg.mozilla.org/mozilla-central/rev/b7486f31f6b1 https://hg.mozilla.org/mozilla-central/rev/16c9cfa0cf84
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Comment 21•13 years ago
|
||
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•13 years ago
|
||
Just tested in Aurora 9.0a2 (2011-10-16) and the bug is still there, worth a bug report then ?
Assignee | ||
Comment 23•13 years ago
|
||
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.
Updated•12 years ago
|
tracking-firefox10:
--- → +
Assignee | ||
Comment 24•12 years ago
|
||
> 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
Assignee | ||
Comment 25•12 years ago
|
||
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
Updated•12 years ago
|
status-firefox10:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•