We need to convert the CF_HTML flavor on the clipboard into our own internal text/html flavor if it's not present.

looks like we're going to need this sooner than later.

ok, needing r/sr, will cc some more people. can now copy html out of IE and paste it into composer.

ccing brade/alecf for some r/sr luvin'

Comment on attachment 81882 [details] [diff] [review] diff -u10 for kathy r=brade

topembed+ as this seems necessary for good Win32 interaction/support

Comment on attachment 81876 [details] [diff] [review] cleaned up patch yuck. Wish that was a unified diff :) hmm.. this alloc/copy step seems like a lot of work just to shift some characters over, especially since I can't figure out where the resulting buffer is freed. Why can't we just headerStart = strchr(data, '<'); NS_ConvertUTF8toUCS2(headerStart) If we must use BuildGeckoHTML, does the length include null termination, or is GetPrimitiveForData smart enough for that?

remove accepting of jpegs from editor code since they don't know what to do with it anyway. now excel97 can paste into mozilla. otherwise same patch as before.

Comment on attachment 81896 [details] [diff] [review] make things a little better with images for composer r=brade

alec, later patches were unified diffs and also showed enough context to show where it was freed. as it ends up, you're right, that's probably a worthwhile simplification. i didn't realize it was going to be so simple at the start. i'll post a new patch.

new, much simpler patch with alecf's comments integrated. needing r/sr

Comment on attachment 81914 [details] [diff] [review] much simpler patch yay, much nicer :) so my only beef is with the commented out AddDataFlavor - might as well just yank it (as you did 3 lines earlier :)) with that, sr=alecf

Comment on attachment 81914 [details] [diff] [review] much simpler patch r=brade (but please cleanup the whitespace changes in nsClipboard.h)

please don't delete that commented out line; it needs to be enabled at some point (when we support pasting of images)

alec: i moved that line 3 down, and commented it out. we don't want to remove it because at some point it will be necessary when we allow drops of images, but where it is now is just plain wrong. when we do finally need it, it belongs at the end of the list, not first.

the telephone is ringing, is that my mother on the phone?

landed on the trunk. waiting for branch approval.

call 1-800-ADT and experience the joy now!

using today's win32 trunk build, on win2k. I'm able to copy/paste & drag/drop tables from Microsoft Excel (version "2002"). I'm able to copy/paste & drag/drop text, images, tables (with both plain text in the cells, as well as images in the cells), & numbered lists from Microsoft Word (version "2002"). Even style (italics, bold, font, font size, color, etc) is carried over in tables and general content; _slick_. When dragging bulleted lists from Word to Composer (or a mail compose window) however, I do notice some garbage trailing the end of the pasted/dragged data (99% reproducible for me, not 100%). The text, bullets, etc are all fine, but, it seems like we're pulling some extra crap (some of it is clearly binary garbage, and sometimes it's ascii data that's probably on my clipboard somewhere) off of the clipboard. I can only repro this w/ bulletted lists, numbered is fine (as is everything else).

I crashed twice (same stacks) while doing some drags, here's the talkback data: - http://climate.netscape.com/reports/SingleIncidentInfo.cfm?dynamicBBID=5854303 - http://climate.netscape.com/reports/SingleIncidentInfo.cfm?dynamicBBID=5853451

*** Bug 117892 has been marked as a duplicate of this bug. ***

*** Bug 92518 has been marked as a duplicate of this bug. ***

Has this landed on the trunk? If yes, please amrk this as Resolved/Fixed, so that QA can verify it. thanks! jrgm - can you verify this on the trunk, and check to see if it introduced any regressions.

On 05-02 trunk build / WinXP-SimpChinese. Problems with Copy/Paste from MS Word into Netscape Composer: 1. I had a crash - talkback ID 5863430 2. Some times the paste string is not showing. 3. Mouse cursor position is at the begining of the string but the end. 4. Some times will occurs some extra garbage get pasted in Composer. From MS Excel to Netscape Composer doesn't has same problem. Note: 1. The problems are not reproduce on 04-24 trunk build on same system. 2. Can not reproduce same problems when Copy/Paste from MS Word into Notepad, IE...etc.

uh, yeah, that's what "landed on the trunk. waiting for branch approval" generally means.

i can't dupe the crash on win2k with ie5. can someone give me a testcase that doesn't involve office XP since i don't have that?

Copy/Paste from MS PowerPoint 2002 SimpChinese / WinXP-SimpChinese and 2002 Japanese / WinXP-JA to Netscape Composer has similar mouse cousor problem, but I haven't seen crash.

ok...i don't have those either.

Actually I tried it on Win2k-SimpChinese with office 97-EN, and I don't see the same problem existing there. However when I copy/paste into Netscape I'll lost the style, e.g. from a PowerPoint with font size 44, and after paste into Composer, then will became a regular size. But with my WinXP, they will keep the font size and style, that's probably cause the problems.

here is the stack trace from talkback 5863430 nsCRT::strlen [nsCRT.cpp, line 180] nsClipboard::GetNativeDataOffClipboard [nsClipboard.cpp, line 424] nsClipboard::GetDataFromDataObject [nsClipboard.cpp, line 555] nsClipboard::GetNativeClipboardData [nsClipboard.cpp, line 800] nsBaseClipboard::GetData [nsBaseClipboard.cpp, line 130] nsHTMLEditor::Paste [nsHTMLDataTransfer.cpp, line 1245] nsPasteCommand::DoCommand [nsEditorCommands.cpp, line 352] nsControllerCommandManager::DoCommand [nsControllerCommandManager.cpp, line 190] nsEditorController::DoCommand [nsEditorController.cpp, line 208] nsXBLPrototypeHandler::ExecuteHandler [nsXBLPrototypeHandler.cpp, line 317] nsXBLWindowHandler::WalkHandlersInternal [nsXBLWindowHandler.cpp, line 312] nsXBLWindowKeyHandler::WalkHandlers [nsXBLWindowKeyHandler.cpp, line 183] nsXBLWindowKeyHandler::KeyPress [nsXBLWindowKeyHandler.cpp, line 199] nsEventListenerManager::HandleEvent [nsEventListenerManager.cpp, line 1656] nsWindowRoot::HandleChromeEvent [nsWindowRoot.cpp, line 182] GlobalWindowImpl::HandleDOMEvent [nsGlobalWindow.cpp, line 777] nsXULDocument::HandleDOMEvent [nsXULDocument.cpp, line 2629] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3488] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3480] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3480] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3480] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3480] nsXULElement::HandleDOMEvent [nsXULElement.cpp, line 3480] nsXULElement::HandleChromeEvent [nsXULElement.cpp, line 4690] GlobalWindowImpl::HandleDOMEvent [nsGlobalWindow.cpp, line 777] nsDocument::HandleDOMEvent [nsDocument.cpp, line 3420] nsGenericElement::HandleDOMEvent [nsGenericElement.cpp, line 1697] PresShell::HandleEventInternal [nsPresShell.cpp, line 5988] PresShell::HandleEvent [nsPresShell.cpp, line 5911] nsViewManager::HandleEvent [nsViewManager.cpp, line 2030] nsView::HandleEvent [nsView.cpp, line 306] nsViewManager::DispatchEvent [nsViewManager.cpp, line 1887] HandleEvent [nsView.cpp, line 83] nsWindow::DispatchEvent [nsWindow.cpp, line 869] nsWindow::DispatchWindowEvent [nsWindow.cpp, line 886] nsWindow::DispatchKeyEvent [nsWindow.cpp, line 2660] nsWindow::OnChar [nsWindow.cpp, line 2810] nsWindow::ProcessMessage [nsWindow.cpp, line 3459] nsWindow::WindowProc [nsWindow.cpp, line 1131] USER32.dll + 0x3a5f (0x77d13a5f) USER32.dll + 0x3b2e (0x77d13b2e) USER32.dll + 0x3d6a (0x77d13d6a) USER32.dll + 0x41fd (0x77d141fd) nsAppShellService::Run [nsAppShellService.cpp, line 451] main1 [nsAppRunner.cpp, line 1472] main [nsAppRunner.cpp, line 1808] WinMain [nsAppRunner.cpp, line 1826] WinMainCRTStartup() kernel32.dll + 0x1eb69 (0x77e5eb69) I think the cursor issue is relative minor. could we open a different bug for that ? ylong- if you don't see the style, that mean the patch does not take effect. mike's patch is design to paste in with style (or I should said the HTML structure)

From the stack trace, the crash problem seems happen here (from my 3minutes code exam. not sure) 472 sfraser 1.43 if ( fe.cfFormat == fileDescriptorFlavor || fe.cfFormat == fileFlavor ) { 473 pinkerton 1.42 NS_WARNING ( "Mozilla doesn't yet understand how to read this type of file flavor" ); 474 pinkerton 1.52 } 475 else { 476 // Get the data out of the global data handle. The size we return 477 // should not include the null because the other platforms don't 478 // use nulls, so just return the length we get back from strlen(), 479 // since we know CF_UNICODETEXT is null terminated. Recall that GetGlobalData() 480 // returns the size of the allocated buffer, not the size of the data 481 // (on 98, these are not the same) so we can't use that. 482 // 483 // NOTE: we are assuming that anything that falls into this default case 484 // is unicode. As we start to get more kinds of binary data, this 485 // may become an incorrect assumption. Stay tuned. 486 PRUint32 allocLen = 0; 487 if ( NS_SUCCEEDED(GetGlobalData(stm.hGlobal, aData, &allocLen)) ) { 488 *aLen = nsCRT::strlen(NS_REINTERPRET_CAST(PRUnichar*, *aData)) * 2; 489 result = NS_OK; 490 } The origional assumption of this part of code is the CF_UNICODETEXT data is null termniated and that could only be assume IF the format is CF_UNICODETEXT. I think pinkerton copy this part code from the case CF_UNICODETEXT. If we hit the CF_HTML code here or any other format which do not guarantee null terminiation , the we may crash here.

well, i don't think we'll crash. we'll probably be off by a few bytes (hence the garbage), but i don't think we'll crash. also, if the string _isn't_ null terminated, there's no way to know the true length of the buffer because the OS returns a length that isn't the real length of the string (see the comment about win98 around that block of code).

a-ha! CF_HTML isn't unicode, it's UTF8, so by falling into that block of code, we'll be off by a factor of two in the length. So why on earth does it work at all!? my debug build is still going, i'll keep you posted...

I think the following line in the line 488 should change from 488 *aLen = nsCRT::strlen(NS_REINTERPRET_CAST(PRUnichar*, *aData)) * 2; to 488 *aLen = allocLen; The following comment may point out the cause 483 // NOTE: we are assuming that anything that falls into this default case 484 // is unicode. As we start to get more kinds of binary data, this 485 // may become an incorrect assumption. Stay tuned. The other way to fix it is to call strnlen (with allocLen/sizeof(PRUnichar) as the 2nd parameter) instead of strlen. Unfortunately, we currently do not have the PRUnichar* version of strnlen in the nsCRT

i think this can be solved by adding |case CF_HTML:| after line 396.

>well, i don't think we'll crash. we'll probably be off by a few >bytes (hence the garbage), but i don't think we'll crash. The fact is ylong DID crash and we have a stack trace from the talkback report. and we know it crash inside nsCRT::strlen if the bytes after it do not have a 0x00 0x00 (two zero bytes) then the strlen will go on and on till it hit a memory which do not belong to the process and crash.

>i think this can be solved by adding |case CF_HTML:| after line 396. Agree, use the current CF_TEXT code to handle CF_HTML. But what happen to the default case? Could other format still hit the default code? Do we need a strnlen to prevent the crash from other cases ?

i have a new patch, but i'm running into issues with nsDependentString that i need jag or scc, but neither are around. i'll look into it more tomorrow.

141866 (issues with nsDependentCString) blocks this, though i can work around it. patch (with workaround) upcoming.

can someone try this and tell me if it works? it works for me, but then again, it always did ;) i'm working with scc so that the string workaround isn't needed, but until then, we're stuck with it. yes, please open a different bug on the insertion point issue. it goes to editor, not to clipboard/dnd. It has nothing to do with the clipboard.

i tried copy/pasting a link from IE into mail composition and after the mail is received , clicking on the link inside the mail body doesn't open a link in separate window, this is not the behavior of copy/paste from Mozilla. same when saving a file from the Composer), used 2002-05-02 trunk build

Removing adt1.0.0 until the new patch has reviews.

> ylong- if you don't see the style, that mean the patch does not take > effect. mike's patch is design to paste in with style (or I should said > the HTML structure) - I saw the style in WinXP from Word or PowerPoint -> Mozilla, but not on Win2k and WinME. (I use office 97)

Removing adt1.0.0, again, until the new patch has reviews.

Comment on attachment 82115 [details] [diff] [review] fix some data len issues r=brade

office97 does _not_ support copying HTML. so don't use office97 to test whether this functionality works.

Comment on attachment 82115 [details] [diff] [review] fix some data len issues sr=kin@netscape.com

I checked in the last patch, today's bits (5/3/02) should have it. try it and let me know if you still see crashes, garbage, etc (again, don't try with office97, you'll only get plain text).

the input string is a fairly long string, every char is in the 7-bit ascii range, with no nulls in it up to the length param. There is one after that in the buffer. Assert is at: NTDLL! 77fa018c() nsDebug::Assertion(const char * 0x10118768 `string', const char * 0x10125fb8, const char * 0x10124fa8 `string', int 1307) line 291 + 13 bytes nsDebug::Error(const char * 0x10118768 `string', const char * 0x10124fa8 `string', int 1307) line 458 + 22 bytes CalculateUTF8Length::write(const char * 0x03b283fd, unsigned int 823) line 1307 + 20 bytes nsCharSinkTraits<CalculateUTF8Length>::write(CalculateUTF8Length & {...}, const char * 0x03b283fd, unsigned int 823) line 559 copy_string(nsReadingIterator<char> & {...}, const nsReadingIterator<char> & {...}, CalculateUTF8Length & {...}) line 90 + 39 bytes NS_ConvertUTF8toUCS2::Init(const nsACString & {...}) line 1328 + 35 bytes NS_ConvertUTF8toUCS2::NS_ConvertUTF8toUCS2(const char * 0x03b283fd, unsigned int 823) line 570 + 38 bytes nsClipboard::GetDataFromDataObject(IDataObject * 0x00153f38, unsigned int 0, nsIWidget * 0x00000000, nsITransferable * 0x03ade5f8) line 610 Like i said, if i don't pass the length, and rely on the null that's in the string, everything is fine. However, i shouldn't have to have that null there.

another problem, office2k doesn't null terminate its html like ie does. and i can't do anything more on this until the dependent string bug is fixed since i need it to work w/out a null. i'll get to this on monday. *sigh*

1. what application should qa test agaist? Word in Office2000? Excel in Office? PowerPoint in Office? IE5? IE5.5? IE6? Word in OfficeXP? Excel in OfficeXP? PowerPoint in OfficeXP? 2. It sounds like CF_HTML is not null terminated. according to the http://msdn.microsoft.com/workshop/networking/clipboard/htmlclipboard.asp you should be able to find the end point from StartHTML and EndHTML in the header to figure out the starting point and the end point of the enclosed html

in the url I mentioned above: >Also, Microsoft&#65533; Internet Explorer places a SourceURL Property in the description section. This allows handlers of CF_HTML to resolve relative links within a file (such as when CF_HTML text is pasted into a DHTML Edit Control host). this could be the answer to the issue marnia mentioned above. But I think we could spin that problem to a seperate bug.

Comment on attachment 82115 [details] [diff] [review] fix some data len issues >+ // end up well. >+ *aLen = strlen ( NS_REINTERPRET_CAST(char*, *aData) ) + sizeof(char); >+ } >+ else >+ *aLen = nsCRT::strlen(NS_REINTERPRET_CAST(PRUnichar*, *aData)) * sizeof(PRUnichar); Did you maybe mean "* sizeof(char)" (to account for the size of a character) or "+ sizeof(char)" (maybe to include the null termination?) Note: "*" vs "+" That might be the cause of the UTF8 assertion, I'm not sure.

i meant '+' to add null termination because of the converter bug, but all this will have to be rewritten to look in the CF_HTML header for the length, since we can't rely on the string anymore.

ok, the last issue was a red herring. the length i was passing was too long. i'll check in this patch as soon as the tree opens. it is good and true. sorry for the hassles.

i have a new patch coming up that backs us up a few stages. CF_HTML will be its own data flavor (that's what editor wanted), so all the nice copying of HTML from IE will be gone. Of course, other things will now work as expected ;) patch forthcoming. cc'ing editor folks.

Ok, this patch, which must be applied to the branch after attachment 82115 [details] [diff] [review], takes us back to a world where we use the text/html flavor by default. Editor will only get CF_HTML when it directly asks for it, via a new flavor, and it will get it as UTF8 with win32 linefeeds and header. r/sr needed.

minor tweaks

Comment on attachment 82641 [details] [diff] [review] tweaks for brade if we fail to FindPlatformHTML, should we return an error? r=brade

last tweaks, some better error handling. r/sr here please.

Comment on attachment 82642 [details] [diff] [review] handle failure of CF_HTML a little better r=brade

Comment on attachment 82642 [details] [diff] [review] handle failure of CF_HTML a little better Do we want to use "application/x-moz-nativehtml" for kNativeHTMLMime so that it's consistent with the line above it? #define kNativeImageMime "application/x-moz-nativeimage" +#define kNativeHTMLMime "application/x-moz-html" pinkerton says the spec was a unclear as to whether order of header data was guaranteed, so I suggested an XXX comment to notate that for this line: + sscanf((char*)*outData, "Version:%f\nStartHTML:%d\nEndHTML:%d", &vers, &startOfData, outDataLen); Is |outData| null terminated? If not, and I'm not sure about this, sscanf could go off into la-la-land if the header wasn't exactly the way we expected it right? A work around could be to just have GetGlobalData() always allocate an extra word at the end of the data and just set it to zero?

kin already gave an sr here. just bulletproofs the alloc to add two extra null bytes at the end so we're guaranteed that any strlen or sscanf's will terminate safely.

Comment on attachment 82675 [details] [diff] [review] bulletproof with guaranteed null termination r=brade

Comment on attachment 82675 [details] [diff] [review] bulletproof with guaranteed null termination sr=kin (really)

landed on trunk. needs adt approval, as well as approval on 141866 for any more work to continue on CF_HTML for embedding (a new bug will be filed and will go to the editor team).

the work for editor to support CF_HTML is http://bugzilla.mozilla.org/show_bug.cgi?id=142855

please make sure iqa (ylong/teruko/andreasb/ji/ruixu/marina) verify it on the trunk before you check into branch thanks.

Checked it on 05-08 trunk build / WinXP-SC: Copy/Paste from MS Word or Powerpoint - compare in comment #24: Don't have crash or not get pasted into Netscape problems, also cursor position is correct, no extra garbled get pasted. However, the pasted string will lost the style. - I used office 2002.

yes yes, the pasted string will lose its formatting from office because we're not doing anything with the CF_HTML now and it falls back to plain unicode. that's ok, don't worry.

adding adt1.0.0+ for checkin to Mozilla 1.0 Branch. Please get drivers approval before checking in and then add the fixed1.0.0 keyword.

*** Bug 144746 has been marked as a duplicate of this bug. ***

Let's get this one on the branch after 1.0 ships. jrgm - Pls verify this on the trunk. thanks!

left hand...right hand...glad to meet the two of you. this isn't going on the branch for a long time, if ever. i'm under orders to bury it until i forget all about it, at which point, it will suddenly be necessary. i'll charge a nice contracting fee at that time.

minusing for 1.0.1. there was mass confusion over what resolving this bug would buy us on the branch w/ out a large chunk of CF_HTML normalization code.

*** Bug 148110 has been marked as a duplicate of this bug. ***

minusing for adt as this shouldn't be on their radar anymore.

Cleaned up whiteboard and keywords - please verify this bug.

This still seems to be a problem. Trying to copy / paste one line from Microsoft Excel 2000 over to Mozilla (Mail Editor) results in no text visible. However if you send the mail it does contain some text but variously garbled. Another thing is that if you copy 2 lines, the top line won't be visible in Mozilla, but however might appear at the recipients end, after the mail has been sent. We're using mozilla on Windows XP Professional, with Microsoft Office 2000, Mozilla version Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv1.3) Gecko/2003030312

Maron Kristofersson (comment 83) -- you are using an old version of mozilla. Please upgrade to version 1.4 RC1 (candidate 1) or newer and try again. If you still see a specific problem with pasting Excel, please file a new bug. Thanks!