Closed Bug 20603 Opened 25 years ago Closed 25 years ago

Copying from a textarea, or plain text composer loses wrapping

Categories

(Core :: DOM: Editor, defect, P2)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sfraser_bugs, Assigned: akkzilla)

Details

Open plaintext composer, or go to a form with a <textarea>. Type some text, with linebreaks (hit return a few times). Select the text and copy it. Now paste into another application. Note that you lose linebreaks, and the pasted text is unwrapped.
Assignee: pinkerton → akkana
in html composer, this works fine as the conversion to platform linebreaks is done before the clipboard sees it. for some reason it's not happening for text areas or plain text compose. akkana told me that it was editor's responsibility to give me platform line endings on a copy, so i'm kicking this to her ;)
To see this problem, you have to copy *existing* text, like the default value for a textarea, or the existing text in plaint text compose. New text with linebreaks is entered with <br>s, which are properly converted to platform linebreaks by teh output system. The output system needs to convert LFs in the text to platform linebreaks, that's one bug. A second bug, that the editor is inserting <br>s in plain text, I will file separately.
Status: NEW → ASSIGNED
We've pinned down what's happening here: copying preformatted text containing embedded newlines (e.g. copy across a few of the lines in the plaintext editor) comes through without the newlines. They should be converted to platform newlines. This is true in yesterday's build, too. Not sure when this broke. Is this dogfood? I'll take a quick look at it regardless, but if it turns out to be tricky I need to know how much time I can spend. I'm inclined to think it should be, for copying out of text areas and plaintext mail compose.
OS: Mac System 8.5 → All
Hardware: Macintosh → All
Target Milestone: M13
It turns out this is because the body isn't included in the selection, and therefore the output system doesn't see the moz-pre-wrap tag and has no way of knowing that the output should be preformatted. It's easy to patch the nsXIFConverter in the clipboard code to pass in OutputPreformatted; but then even non-preformatted text is treated as formatted. We need to have some way of knowing when text is supposed to be treated as pre. At one time, I had a change to nsHTMLDocument which extended nsDocument::IsInSelection to make the document's body always be part of XIF output even for a selection. I could have sworn that I checked that in, but there's no record of it in CVS (I must be going crazy). Maybe that needs to be done again. It's a big enough change that I'd better hold off unless someone decides this is dogfood. Marking M13.
I remembered that it may be hard to get things in after M12, and I found an easy way of implementing nsHTMLDocument::IsInSelection(), so I did it. Who wants to review it?
I've checked in the changes to make the body node part of the selection. The changes to nsHTMLToTXTSinkStream.cpp are more complicated, because the "intelligent wrapping" path ignores newlines (it shouldn't, at least in the case where mPreformatted is set) but sending all mPreFormatted output through the non-intelligent path also has problems.. CC'ing Daniel in case he's still interested in this code.
A second manifestation of the same problem is that when filling textareas in a form from bookmark text, we insert extra linebreaks into the text.
Priority: P3 → P2
This bug is also causing us to post to newsgroups with lines that are too long for the news server to handle. See mscott's post, <384DD27E.6010800@netscape.com> , and notice that the server (we think) has added '!' in two places where the lines end. I think this is a pretty serious bug.
I've looked at it and think I have a patch. I don't see the problem anymore with this patch but I notice that the tests for the sink reports things at severly broken, which they weren't some weeks ago. Is this a known regression? This is the simple patch that fixes the problem: Index: nsHTMLToTXTSinkStream.cpp =================================================================== RCS file: /cvsroot/mozilla/htmlparser/src/nsHTMLToTXTSinkStream.cpp,v retrieving revision 3.42 diff -u -r3.42 nsHTMLToTXTSinkStream.cpp --- nsHTMLToTXTSinkStream.cpp 1999/11/24 00:56:03 3.42 +++ nsHTMLToTXTSinkStream.cpp 1999/12/09 21:08:31 @@ -1209,7 +1208,8 @@ mInWhitespace=PR_FALSE; } else { if(mInWhitespace && (nextpos == bol) && - !(mFlags & nsIDocumentEncoder::OutputPreformatted)) { + !(mFlags & nsIDocumentEncoder::OutputPreformatted) && + !mPreFormatted) { // Skip whitespace bol++; continue;
I tried that simple fix, but it doesn't fix the problem. It's fine on <br>s in the tree, but it doesn't pass newlines through. To see this, run the plaintext editor on its init page (unfortunately Charley just changed EditorNewPlaintext() to bring up about:blank, so you have to edit EditorCommands.js and change the about:blank in EditorNewPlaintext to chrome://editor/content/EditorInitPagePlain.html), select across several lines of that text, and then paste into something that takes plaintext (e.g. a shell window or a plaintext editor) and notice that the line breaks don't get through.
I had some other (old) changes in the file too. Maybe they are needed for the patch to work. I have no time to verify that they don't break anything but you're welcome to try them. This is the full diff: Index: nsHTMLToTXTSinkStream.cpp =================================================================== RCS file: /cvsroot/mozilla/htmlparser/src/nsHTMLToTXTSinkStream.cpp,v retrieving revision 3.42 diff -u -r3.42 nsHTMLToTXTSinkStream.cpp --- nsHTMLToTXTSinkStream.cpp 1999/11/24 00:56:03 3.42 +++ nsHTMLToTXTSinkStream.cpp 1999/12/09 21:08:31 @@ -443,13 +443,6 @@ if (type == eHTMLTag_body) { - // body -> can turn on cacheing unless it's already preformatted - if(!(mFlags & nsIDocumentEncoder::OutputPreformatted) && - ((mFlags & nsIDocumentEncoder::OutputFormatted) || - (mFlags & nsIDocumentEncoder::OutputWrap))) { - mCacheLine = PR_TRUE; - } - // Try to figure out here whether we have a // preformatted style attribute. // @@ -463,7 +456,6 @@ (-1 != value.Find("-moz-pre-wrap"))) { mPreFormatted = PR_TRUE; - mCacheLine = PR_TRUE; PRInt32 widthOffset = value.Find("width:"); if (widthOffset >= 0) { @@ -489,9 +481,16 @@ } } else { mPreFormatted = PR_FALSE; - mCacheLine = PR_TRUE; // Cache lines unless something else tells us not t o } + // body -> can turn on cacheing unless it's already preformatted + if(!mPreFormatted && + !(mFlags & nsIDocumentEncoder::OutputPreformatted) && + ((mFlags & nsIDocumentEncoder::OutputFormatted) || + (mFlags & nsIDocumentEncoder::OutputWrap))) { + mCacheLine = PR_TRUE; + } + return NS_OK; } @@ -1209,7 +1208,8 @@ mInWhitespace=PR_FALSE; } else { if(mInWhitespace && (nextpos == bol) && - !(mFlags & nsIDocumentEncoder::OutputPreformatted)) { + !(mFlags & nsIDocumentEncoder::OutputPreformatted) && + !mPreFormatted) { // Skip whitespace bol++; continue;
Target Milestone: M13 → M14
Unfortunately, an event handling bug has surfaced which means I probably won't have time to fix this for M13. Since this doesn't seem to be something that's getting in people's way very much, moving this to M14.
I think that some of the intervening changes to plaintext output may have fixed this bug. I don't see this any more on Linux. Would someone please try on Mac and see if you still see the problem? Thanks!
Looks like this is fixed on my system and Simon's. Marking as fixed ... if anyone is still seeing this, please reopen.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
verified in 2/4 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.