86450 - pasting plaintext into text area loses significant whitespace

Status: VERIFIED FIXED

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

Description

[Chris Waterson]

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

Comment 1

[Chris Waterson]

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?

Comment 2

[rubydoo123]

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

Comment 3

[Chris Waterson]

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

[rubydoo123]

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.

Comment 5

[rubydoo123]

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

Comment 6

[rubydoo123]

moving to 9.3

Comment 7

[kinmoz]

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.

Comment 8

[Chris Waterson]

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).

Comment 9

[kinmoz]

Attachment: Test case.

Comment 10

[rubydoo123]

this has a wide impact on folks who copy/paste

reviewed and approved

Comment 11

[kinmoz]

Attachment: Patch that fixes the bug. (rev 1)

Comment 12

[kinmoz]

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.

Comment 13

[kinmoz]

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

Comment 14

[kinmoz]

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.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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

Comment 16

[Kathleen :Brade]

Attachment: patches for drag & drop

Comment 17

[kinmoz]

Attachment: Updated/Combined diffs from brade and kin.

Comment 18

[kinmoz]

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.

Comment 19

[Simon Fraser [no longer active]]

+      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'.

Comment 20

[kinmoz]

Attachment: Combined patch. (Rev 2, addresses sfraser's comments)

Comment 21

[Simon Fraser [no longer active]]

sr=sfraser

Comment 22

[kinmoz]

Fix checked into the TRUNK with the following CVS comment:


Fixes from [e-mail] and [e-mail]
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=[e-mail],[e-mail] sr=[e-mail]

Comment 23

[chris hofmann]

lets get this on the branch

Comment 24

[kinmoz]

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=[e-mail],[e-mail]
sr=[e-mail]
a=[e-mail],[e-mail] (pdt)

Comment 25

[sujay]

verified on trunk 7/17 build. removing vtrunk keyword

will verify on the branch now.

Comment 26

[sujay]

verified in 7/17 branch. looks good.

marking verified.

Comment 27

[Richard Donkin]

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.