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.