pasting plaintext into text area loses significant whitespace

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
Editor
P3
major
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Chris Waterson, Assigned: kinmoz)

Tracking

({regression})

Trunk
mozilla0.9.3
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [paste special] [in trunk] pdt+)

Attachments

(5 attachments)

(Reporter)

Description

17 years ago
To reproduce, select some text from a patch in bugzilla somewhere, and paste it
into the ``Description'' field. Note that significant whitespace is lost; e.g.,

x  indent level 1
x      indent level 2
x          indent level 3

becomes:

x indent level 1
x indent level 2
x indent level 3
(Reporter)

Comment 1

17 years ago
This is a dogfood issue for me. I have to comment on patches all day long
because of my illustrious designation as a super-reviewer. Putting the
whitespace _back into_ the patches I'm commenting on is not where I'd like to be
spending my time. Could we please fix this for mozilla-0.9.2?
Keywords: mozilla0.9.2, nsdogfood, regression

Comment 2

17 years ago
that paste would have to be wrapped into a pre to preserve the whitespace, 
whitespace is stripped out everywhere else except a pre element.

Updated

17 years ago
Whiteboard: [paste special]
(Reporter)

Comment 3

17 years ago
I'm not pasting from HTML. I'm pasting from a plain-text document, that is, a 
bugzilla attachment whose MIME type is text/plain.

Comment 4

17 years ago
in that case, there is a problem since pre isn't allowed in a textarea, 
textareas can only contain pcdata. It does not matter where you are pasting 
from -- plaintext or html.
(Reporter)

Updated

17 years ago
Summary: pasting into text frame loses significant whitespace → pasting plaintext into text area loses significant whitespace

Comment 5

17 years ago
I apologize, I based my comments on the constructs of the DTD and not on the 
implementation behavior of how we process the content. Kin is picking this one 
up and will determine if it is a editor issue or a layout copy issue
Assignee: beppe → kin

Comment 6

17 years ago
moving to 9.3
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3
(Assignee)

Comment 7

17 years ago
Hey Chris, I'm unable to reproduce this problem with my Win32 Debug build from 
06/20/01.

Was there a patch/bug in particular you were using when you encountered this 
problem? I ask because I know that line breaks in patches are different, 
depending on what platform a person was using when he/she attatched the patch to 
a bug.
Status: NEW → ASSIGNED
(Reporter)

Comment 8

17 years ago
Sure, I'll look for the specific bug that caused the problem and attach a test 
case here.

I don't know if it's related (but from an idjit's point of view, it sure seems 
to be), but... If I copy whitespace-indented text *out* of a text field (e.g., 
to another browser window, or to another app), I lose all my formatting (at 
least on Linux).
(Assignee)

Comment 9

17 years ago
Created attachment 40129 [details]
Test case.

Comment 10

17 years ago
this has a wide impact on folks who copy/paste

reviewed and approved
Keywords: nsBranch
(Assignee)

Comment 11

17 years ago
Created attachment 40339 [details] [diff] [review]
Patch that fixes the bug. (rev 1)
(Assignee)

Comment 12

17 years ago
The bug is actually on the Copy side of things where the neccessary flags for 
the document encoder weren't being set during init time.

I just attatched a patch that fixes this problem.

Note that there is some code in nsDocumentEncoder::SetSelection() that is 
responsible for turning a "text/html" encoder into a "text/plain" encoder. I 
didn't touch it because there seems to be code elsewhere that relies on this 
behavior. But I'd like to remove it someday since it seems like an odd place to 
switch mimeTypes.
Whiteboard: [paste special] → [paste special] FIX IN HAND, need r= and sr=
(Assignee)

Comment 13

17 years ago
*** Bug 69024 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 14

17 years ago
I did some Copy/Paste testing with Composer, PlainText Editor, HTML MsgCompose, 
PlainText MsgCompose, and TextAreas and things seem to work fine.

I did notice that D&D of text from the various PlainText editors has the same 
problem ... but that goes through a separate code path.
(Assignee)

Updated

17 years ago
OS: Windows 2000 → All
Hardware: PC → All
*** Bug 90082 has been marked as a duplicate of this bug. ***

Comment 16

17 years ago
Created attachment 41936 [details] [diff] [review]
patches for drag & drop
(Assignee)

Comment 17

17 years ago
Created attachment 41977 [details] [diff] [review]
Updated/Combined diffs from brade and kin.
(Assignee)

Comment 18

17 years ago
Ok, I just attatched a combined diff of the changes from brade and I. I've
reviewed her diffs, and she's reviewed mine, so now we're looking for an sr=
from sfraser to land this puppy on the trunk.
Whiteboard: [paste special] FIX IN HAND, need r= and sr= → [paste special] FIX IN HAND, sr=

Comment 19

17 years ago
+      mimeType.AssignWithConversion(kUnicodeMime);
+    }
+    else
+      mimeType.AssignWithConversion(kHTMLMime);

Can we use NS_LITERAL_STRING here?

+        nsCOMPtr<nsIFormatConverter> htmlConverter;
+        rv = nsComponentManager::CreateInstance(kCHTMLFormatConverterCID, 
nsnull, NS_GET_IID(nsIFormatConverter),
+                                                getter_AddRefs(htmlConverter));

use do_CreateInstance here?

+        if (NS_FAILED(rv)) return rv;
+        if (!htmlConverter) return NS_ERROR_OUT_OF_MEMORY;
+

You can safely assume that CreateInstance never returns NS_OK and a null value.

Same comments for the plain text data transfer.

+    if (atom.get() == nsHTMLAtoms::input ||
+        atom.get() == nsHTMLAtoms::textarea)
+    {
+      *aIsPlainTextContext = PR_TRUE;
+      break;
+    }
+    else if (atom.get() == nsHTMLAtoms::body)

No need for 'else' after 'break'.

(Assignee)

Comment 20

17 years ago
Created attachment 42123 [details] [diff] [review]
Combined patch. (Rev 2, addresses sfraser's comments)

Comment 21

17 years ago
sr=sfraser
(Assignee)

Updated

17 years ago
Whiteboard: [paste special] FIX IN HAND, sr= → [paste special] FIX IN HAND, reviewed
(Assignee)

Comment 22

17 years ago
Fix checked into the TRUNK with the following CVS comment:


Fixes from kin@netscape.com and brade@netscape.com
for bug #86450: pasting plaintext into text area loses significant whitespace

    content/base/src/nsDocumentEncoder.cpp    revision 1.51
        - Look for -moz-pre-wrap in the correct attribute ("style").

    editor/base/nsHTMLDataTransfer.cpp         revision 1.17
    editor/base/nsPlaintextDataTransfer.cpp    revision 1.9
        - Set the correct mime-type on the encoder and transfer data.
        - Pass correct init flags to encoder.

    layout/base/src/nsCopySupport.cpp    revision 1.9
    layout/base/src/nsCopySupport.h:     revision 1.3
        - Added IsPlainTextContext() method.
        - Modified HTMLCopy() to pass correct init flags and
          mime-type to the encoder during plaintext copy.

r=kin@netscape.com,brade@netscape.com sr=sfraser@netscape.com
Keywords: vtrunk

Updated

17 years ago
Whiteboard: [paste special] FIX IN HAND, reviewed → [paste special] [in trunk]

Comment 23

17 years ago
lets get this on the branch
Whiteboard: [paste special] [in trunk] → [paste special] [in trunk] pdt+
(Assignee)

Comment 24

17 years ago
Fix checked into the MOZILLA_0_9_2_BRANCH:

    content/base/src/nsDocumentEncoder.cpp    revision 1.49.38.1
    editor/base/nsHTMLDataTransfer.cpp        revision 1.15.2.1
    editor/base/nsPlaintextDataTransfer.cpp   revision 1.7.28.1
    layout/base/src/nsCopySupport.cpp         revision 1.7.2.1
    layout/base/src/nsCopySupport.h:          revision 1.2.56.1

r=kin@netscape.com,brade@netscape.com
sr=sfraser@netscape.com
a=chofmann@netscape.com,clayton@netscape.com (pdt)

Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 25

17 years ago
verified on trunk 7/17 build. removing vtrunk keyword

will verify on the branch now.
Keywords: vtrunk

Comment 26

17 years ago
verified in 7/17 branch. looks good.

marking verified.
Status: RESOLVED → VERIFIED

Comment 27

17 years ago
Thanks for fixing this, a lot of TWiki users and Wiki users generally will be 
very happy - see http://twiki.org/ and http://www.c2.com/cgi/wiki for more 
information, these are tools for collaborative web page creation that rely on 
space indentation for formatting.
You need to log in before you can comment on or make changes to this bug.