17565 - [DOGFOOD] pasting form data turns loses newlines

Status: VERIFIED FIXED

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

Description

[Warren Harris]

I've noticed that if you cut test from a form field in mozilla and paste it into
a form field in 4.x, all the carrage-returns get turned into little boxes.

Comment 1

[rubydoo123]

setting to M14 and assigning to sfraser for investigation

Comment 2

[Warren Harris]

Why M14 if this is dogfood?

Comment 3

[leger]

Putting on the PDT+ radar.

Comment 4

[rubydoo123]

it wasn't PDT+ when I pushed it out to M14 -- now that it is, I'm setting it to
M12.

Comment 5

[Akkana Peck]

I'll investigate.

Comment 6

[Akkana Peck]

Warren: are you still seeing this?

Comment 7

[rubydoo123]

this particular bug has not been investigated yet, akkana will update the bug as
she digs into the issue.

Comment 8

[Akkana Peck]

Kin just let me try this on his Windows build (we typed several lines into a
bugzilla Comments textarea, copied it, pasted to a similar textarea in 4.x, then
tried the reverse as well), and it looks like both directions are working fine
now.  Marking fixed -- Warren, if you're still seeing this, please reopen with
more information on how to reproduce it.

Comment 9

[Warren Harris]

Although I'm not seeing little boxes anymore, it's still not working right.

If I cut text out of a mail message in 4.x and paste it into a text edit field,
CRs are preserved. But if I cut text out of a seamonkey text edit field and
paste it into a 4.x text edit field, all the CRs are taken out, e.g. rather
than this:

*** shadow/17565        Wed Nov 10 11:46:22 1999
--- shadow/17565.tmp.12284      Thu Nov 11 11:35:07 1999
***************
*** 3,10 ****
  Version: other
  Platform: PC
  OS/Version: Windows NT
! Status: ASSIGNED
! Resolution:
  Severity: normal
  Priority: P1
  Component: Editor
--- 3,10 ----
  Version: other
  Platform: PC
  OS/Version: Windows NT
! Status: RESOLVED
! Resolution: FIXED
  Severity: normal
  Priority: P1
  Component: Editor

I get this:

*** shadow/17565 Wed Nov 10 11:46:22 1999 --- shadow/17565.tmp.12284 Thu Nov 11
11:35:07 1999 *************** *** 3,10 **** Version: other Platform: PC
OS/Version: Windows NT ! Status: ASSIGNED ! Resolution: Severity: normal
Priority: P1 Component: Editor --- 3,10 ---- Version: other Platform: PC
OS/Version: Windows NT ! Status: RESOLVED ! Resolution: FIXED Severity: normal
Priority: P1 Component: Editor

Comment 10

[Akkana Peck]

Yes, I see this too (yay, a reproducible case!)  Somehow when pasting into the
editor, the newlines are preserved, but not when pasting into an external app.
I'm investigating why.

Comment 11

[Akkana Peck]

Okay, I think I have a handle on what's happening.  The nsHTMLToTXTSinkStream
(which does the wrapping) has to ignore newlines and whitespace unless it's
inside a PRE, or inside a style element on the body node like _moz-pre-wrap, in
which case, newlines are significant.  It and the nsIDocumentEncoder also have a
flag, OutputPreformatted, to tell it to treat the output as preformatted even if
it doesn't see one of these tags (which it won't in the case of a copy, since
it's getting only a part of the document, not including the body tag with the
style element on it).

The problem is that the XIF converter in the clipboard code is structured not to
go through the document encoder, and it doesn't set any flags in the output sink
(just takes the default), and doesn't have any provision for passing in flags to
be handed off to the output sink, so there's no way for a plaintext object (like
the plaintext editor or a gfx text area) to tell the clipboard that we should be
in preformatted mode.

I think this used to work; but I also remember noticing that the editor used to
insert <br> tags for returns even in plaintext mode.  Cc'ing Joe, maybe he knows
about recent changes.

I'm not sure what the right solution to this is; we could
1. Change the clipboard code to allow setting flags (cc'ing pinkerton)
2. Change nsDocument to include the body tag in all selections, then change the
html sink stream to filter it out when outputting a selection.
3. Change the editor to go back to inserting <br> tags when it encounters a
newline in InsertText.  If this is a recent change, maybe changing it back would
be the easiest.  Joe?

Comment 12

[Akkana Peck]

I tried writing the nsHTMLDocument change to always output the body as part of
the selection.  It did fix the problem (modulo next paragraph) but I'm not sure
if it's safe in other contexts, like html paste.  Also, it requires casting away
a const, which may not be considered kosher (have to talk to layout people about
policies on that).

then I also had to change nsHTMLToTXTSinkStream::Write to not to smart wrapping
whenever mPreFormatted is set: previously, it required !(mPreFormatted &&
!mWrapCol).  I don't think switching on whether wrap col is set is safe, since
it defaults to 72.  Daniel?  Any idea why this is the way it is?  Alternately
(if there's some reason for checking mWrapCol here), we could change the
intelligent wrapping code to respect newlines.

Comment 13

[Akkana Peck]

Attachment: layout patch to make body part of selection

Comment 14

[Akkana Peck]

Attachment: patch to respect newlines whenever mPreFormatted is set

Comment 15

[Akkana Peck]

Removing the check for mWrapColumn in nsHTMLToTXTSinkStream causes mail not to
wrap.  We need a smarter solution there (but people concerned with this bug can
still use the small patch for testing; there's a separate bug on mail wrapping
problems).

Comment 16

[Akkana Peck]

BTW, the original subject of this bug, "carriage returns appear as little boxes"
(or sometimes musical notes or other characters, depending on your font) has
reappeared, and is now bug 18409.  It seems to be an issue with the Windows
clipboard.  Anyone still seeing that problem, add yourself to that bug since
this one has mutated into something else.

Comment 17

[rubydoo123]

using the win32 build 1999111608, this is what I tried:
1. went to bugzilla in seamonkey and accessed this bug, typed in bogus text and
allowed it to wrap on its own.

2. Highlighted the text, selected copy (ctrl+c) and pasted into a 4.7 window.
What I did was save a bugzilla report to the desktop, altered the form ACTION
attribute to mailto.
RESULT: the text wrapped within the 4.7 window based on the textarea size within
the 4.7 window -- which is what I expected.

3. Opened Word and pasted the bogus text from seamonkey.
RESULT: the text wrapped within Word based on the margins settings -- which is
what I expected.

4. Returned to the seamonkey app, entered hard returns in the bogus text to
force newlines. Highlighted the text, selected copy (ctrl+c) and pasted into the
4.7 window.
RESULT: the text wrapped within the 4.7 window based on the hard returns that
I entered -- which is what I expected.

5. Pasted the bogus text into Word.
RESULT: the text wrapped within Word based on the margins set in Word, one
interesting note - the hard returns came over as <BR>s.

6. I did one more test, I used Shift+Enter (soft return). Highlighted the text,
selected copy (ctrl+c) and pasted into the 4.7 window.
RESULT: the text wrapped within the 4.7 window based on the soft returns that I
entered -- which is what I expected.

7. Pasted the bogus text into Word.
RESULT: the text wrapped within Word based on the margins set in Word, the soft
returns came over as <BR>s.

The whiteboxes did not display. So, the short answer is -- I don't see this
happening in windows using a current build.

I'm marking this little guy fixed -- Warren, if you see this again, please let
me know.

Comment 18

[Warren Harris]

The "soft returns" should translate into returns too, not <br>s. Yes, this is
lossy, but it's what's expected. Reopening.

Comment 19

[rubydoo123]

but the result is the same as the hard returns -- so why is this wrong?

Comment 20

[buster]

sounds like we're advertising html on the clipboard when doing a copy from a
plain text editor.  That's wrong.  Even though we're editing html under the
hood, a copy operation should look at the eEditorPlaintextBit flag, and if it's
set we should only put plain text on the clipboard. This should be true of gfx
text controls, plain text editor app, plain text mail compose, etc.

Comment 21

[Akkana Peck]

Hmm, interesting point, Steve.  Right now, the editor doesn't do its own copy;
it just calls the pres shell's DoCopy().  The pres shell doesn't know whether it
has a plaintext editor or not, and probably isn't the right place to be making
this determination.

So perhaps we have to duplicate the code in PresShell::DoCopy and make a
separate editor version, which may or may not advertise html depending on the
editor type.  That's quite easy to do; is it the right thing?

BTW, I'm confused by all the "soft return" talk in the last few comments.  I
didn't think we implemented "soft returns" -- if we do, what do we put into the
document?  Also, I wasn't clear when Beth was talking about the plaintext editor
and when the html editor, or which sort of "soft newlines" warren expects to see
translated into NS_LINEBREAK and under what conditions.

I'd love to fix this bug, if I could only figure out what the bug is and what
people expect to have happen. :-)  Can someone please provide a very concrete
before-and-after scenario, telling me what's in the document (from
Debug->DumpContentTree or Debug->OutputHTML in either plaintext or html editor),
what goes into plaintext output (Debug->OutputPlain), and why you think this is
wrong and what should go into the output?  Or, if OutputPlain is correct but
pasting the result into an editor (specify plain or html) turns it into
something bad, I'd like to know that, too.

This is still considered to be Windows only, right?

Did anyone ever try the patches I posted to this bug, and did they help?

Comment 22

[rubydoo123]

The original complaint on this bug, evolved around copying text entered into a
text field -- like the comment entry field used to enter this comment -- from
seamonkey into a text field in 4.7. That behavior does not exist anymore -- it
has been fixed.

CRs are honored when you copy and paste into 4.7, they are also honored when
copied into notepad.

No whieboxes replace CRs

This that issue is resolved, I am marking this resolved.

If there are other issues that need to be addressed, then open a new bug.

Comment 23

[sujay]

verified in 11/29 build.