18409 - [Dogfood] Pasting from clipboard causes extraneous characters for any CR's in the data

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P1)

Description

[Katsuhiko Momoi]

** Observed with 11/9/99 Win32 build (1999110911) **

This is a left over bug from Bug 15465. In that bug, rhp fixed the problem of not quoting 8-bit characters correctly.
This problem was mentioned in that bug but it has nto been fixed along with Bug 15465. Therefore there is now need to
create a new bug for it.

Problem description:

1. Place the Smoketest mailbox obtained at the above URL into your POP mail directory (or copy to IMAP from POP
   Mail directory)
2. Pick out any one of the 5 msgs in it and select it.
3. Under Plain Text send option, click on Reply button.
4. Observe that the quoted text body contains extraneous characters at the beginning or the end of each line
   of the quoted text. Depedening on the language of Windows OS, you may see a square character or
   a 1/8th muscial note symbol.

This is an eyesore and real detriment in composing Reply msgs and must be eliminated.

Comment 1

[Jean-Francois Ducarroz]

Reassign to rhp

Comment 2

[bobj]

This also happens with ASCII only messages.  There is no need to include 8-bit
chars.  I don't think you need to use Kat Momoi's test cases.  Probably any
1 line message will cause this.

Comment 3

[leger]

Putting on PDT+ radar.

Comment 4

[rhp (gone)]

I really don't know what to do with this one. Just take a look at the image I
will attach to this bug. It is doing exactly what you describe and I don't see
ANY garbage characters.

Naoki, any help here would really be appreciated.

- rhp

Comment 5

[rhp (gone)]

Attachment: This is what the compose window looks like on my system

Comment 6

[Katsuhiko Momoi]

I've asked erik to comment on this.

Comment 7

[nhottanscp]

Attachment: screen shot of quoted plain text using today's M12 release build

Comment 8

[Erik van der Poel]

Kat is seeing a strange music glyph at the end of each line. Rich is not seeing
any strange glyph there. I suspect that this is because Kat has more fonts on
his system, so that the font engine could find a glyph for that strange char.
On Rich's system, it wouldn't show anything since his system doesn't have any
fonts with that glyph.

So, I suspect that the code is going past the end of each line into a piece of
memory that happens to have some garbage code in it that looks like music on
the screen. Kinda like the music though...

Comment 9

[rhp (gone)]

I don't doubt you are seeing this, but I'm just saying that I can't fix what I
can't reproduce...please help.

- rhp

Comment 10

[Erik van der Poel]

I can reproduce this on my NT machine with a debug build. If somebody could tell
me where the newlines (0D 00 0A 00) are being converted to "> " for mail
quoting, I can try to set a breakpoint there and see what's going on.

Comment 11

[rhp (gone)]

Akkana, can you answer Erik's question?

Comment 12

[Akkana Peck]

No, I can't -- I thought the reply-generating code was your code.  The output
code never converts newlines to "> " in doing plaintext-to-plaintext (or if it
did, it would be a bug).

There are lots of useful options in the Debug menu (e.g. Dump Content Tree,
Output HTML, Output Text) of editor and mail compose windows which would be
useful to someone who's actually seeing this for pinning down this bug and
figuring out when it's happening.

Comment 13

[Akkana Peck]

Wait, are people seeing this with the recent change (today) to nsMsgCompose.cpp
which actually calls the editor's InsertAsQuotation code?  If that's the case,
then the > gets inserted in nsInternetCiter::GetCiteString.  I didn't think the
old mime code was using that code at all, though, and I thought this was a bug
people were seeing prior to today.

Anyone seeing it prior to today should update nsMsgCompose.cpp and
nsHTMLToTXTSinkStream.{h,cpp} and see if they still see it.

Comment 14

[rhp (gone)]

Actually, this has been a problem for a while...before the InsertAsQuotation
change.

- rhp

Comment 15

[Akkana Peck]

Then people who were seeing this should definitely check again -- the code path
by which plaintext replies are generated has changed completely.

Comment 16

[nhottanscp]

Attachment: the same problem in the plain text editor

Comment 17

[nhottanscp]

Using today's pull, I can see the music symbols in the plain text editor when I
paste or paste as quote more than one line.

Comment 18

[rhp (gone)]

Well, we should probably reassign this to someone in the "editor group?". I
don't have any code for copy & paste so it looks like an issue with line
endings.

Any ideas for a reassignment

- rhp

Comment 19

[nhottanscp]

This also happens in the HTML editor. Reassigning to akkana. This is
reproducible on my windows machine so let me know if you want me to debug
something.

Comment 20

[Akkana Peck]

I need to know whether this is reproducible using a build pulled after 1pm on
11/11 ("pulled today" usually means in the morning) and what the document looks
like according to Debug->DumpContentTree and Debug->OutputText.  Also, how to
reproduce it in the HTML editor (that's a new one that I hadn't seen anyone
mention before).

Comment 21

[nhottanscp]

No, it was pulled before 1pm 11/11. I pulled again this morning and building
now. I will post the test results after my build completes.

Comment 22

[nhottanscp]

Attachment: a screen shot, after html editor paste, used today's win32 release build

Comment 23

[nhottanscp]

Attachment: debug dump, html editor paste, used today's win32 release build

Comment 24

[nhottanscp]

I attached the data. I used today's win32 build (M12). Also happened on my local
build (pulled around 10 am today). HTML problem is reproducible the same way as
plain text (paste or paste as quote multiple lines).

Comment 25

[Akkana Peck]

Naoki and the editor team spent some time this morning diagnosing this.  The
problem is returns are getting inserted into the DOM tree by InsertText, but the
content tree is never supposed to have returns (all platform line break
characters are supposed to be converted to \n -- normally the parser does this,
but the parser isn't involved in InsertText).  The editor team discussed a
solution where we ignored CR, but that would break the Mac since that's its only
line break character.  We came to the conclusion that the best solution is for
the platform-specific parts of the clipboard code to stop including returns in
the nsString passed back to callers requesting the selection.

It's not clear what the Mac does, or whether this is a problem on Mac, but
nobody has reported a lack of line separators in text areas on the Mac, so my
guess is that the Mac already does this correctly, and that this is a matter of
fixing the Windows clipboard code to do something similar (probably just do a
  string.StripChar('\r');
on the nsString before passing it back).

Passing to Pinkerton and Rods, the windows clipboard people.

Comment 26

[Katsuhiko Momoi]

I just looked at a Mac build from 11/11/99 (labeled M12).
It has a problem of not drawing the text body at all until you swipe
your cursor over the text body area. Once the text is drawn, you can see
that:

Under plain text send, it does not look like Mac is quoting
the original with line breaks. The lines are bunched without
reflecting the original line breaks.

I don't see extraneous characters, however.

Comment 27

[Akkana Peck]

Unfortunately, a release build from 11/11 doesn't tell us much because that's
just before the code path for mail replies changed completely.

Comment 28

[Erik van der Poel]

I tried the 11/12 build on Windows, and it crashed when I tried to reply to a
message. Anyone else seeing this?

Comment 29

[Katsuhiko Momoi]

Most people I know are crashing on reply with today's build.

Comment 30

[Mike Pinkerton (not reading bugmail)]

in what cases should the clipboard code strip returns and when should it not? it
seems to only a problem with quoting things, and i can't tell the difference
between that case and normal text. I really don't want to put code into the
clipboard that knows specifically about quoting in mail messages. that just rubs
me the wrong way.

sorry, but i'm coming into this very very late.

Comment 31

[Akkana Peck]

Basically, when the clipboard is asked for a string which is going to be pasted
inside our app, it should remove the carriage returns and use only linefeeds,
because that's the newline character the DOM recognizes inside the content tree.
Strings copied from within our app should not have returns in them (if they do,
that's a bug which I need to investigate) but in any case, strings coming from
other apps will probably have returns which need to be stripped on a Paste so
that they don't get inserted into the content tree.

Comment 32

[Mike Pinkerton (not reading bugmail)]

but i thought this text that was copied was actually from our app. are you saying
the stripping of the returns in the clipboard is a separate problem/bug?

Comment 33

[nhottanscp]

The problem was originally found in mail quote then later also observed in the
editors. I think fixing the mail quote problem
needs a separate change in mail/news code (clipboard change fixes the editor
paste problem). So this bug could be separated
into two.

Comment 34

[Peter Trudelle]

This is hardly critical. targetting M12, p2

Comment 35

[rhp (gone)]

There are actually 2 bugs here...one is the issue of the clipboard and the
other is quoting on mail compose. Basically, I convert all CRLF's to LF's and
then CR's get converted to LF's (for Mac). I have a fix for the mail compose in
my tree and I hope to get that checked in tomorrow. I will create a separate
bug for this assigned to me and I will reference this bug.

- rhp

Comment 36

[Mike Pinkerton (not reading bugmail)]

dunno what to do about this one, pushing it out.

Comment 37

[nhottanscp]

I think the remaining changes needed in our native clipboard code are follwing.
windows - convert CRLF's to LF's
macintosh - convert CR's to LF's
unix - no change

Comment 38

[Mike Pinkerton (not reading bugmail)]

Ok, just for my own sanity, let me get this straight. Our internal editor code
(and the DOM) only understands LF. However, text coming from other apps on a
given platform may contain the platform-specific line endings (CRLF, for example,
on win32). As a result, the native clipboard impls need to convert things into a
common format (only LF) when the text is requested.

Is this correct?

What data flavors will this have to be done for? I assume text/plain, text/
unicode, and text/html?

Comment 39

[Mike Pinkerton (not reading bugmail)]

this is next on my list to fix. marking p1. i now see this copying text from
codewarrior and pasting into editor, so it's easy for me to get a handle on when
i have a fix.

Comment 40

[Mike Pinkerton (not reading bugmail)]

dependency on two bugs regarding how line termination is actually handled w/in
our app. In the process, simon will check in code into XPCOM that will break out
the line termination conversion code so we don't have to write it over and over
again. Once that code is in place, and all parties agree on the "right way to do
things," this should be a snap to implement for clipboard/d&d.

cc'ing simon.

Comment 41

[Mike Pinkerton (not reading bugmail)]

fixes checked in for all platforms.

Comment 42

[Jerry Baker]

This is till happenning to me with 1999120608. This also happens in the URL
location box in the browser. Pasting is causing weird characters at the
beginning or end of strings, or sometimes it pastes the contents of the
clipboard twice with one paste operation.

Win98SE

Comment 43

[Jerry Baker]

Attachment: Mail window with bad paste behavior

Comment 44

[Mike Pinkerton (not reading bugmail)]

this works for me on NT, wondering if it's a 98 vs NT problem. reopening for the
time being.

the double paste bug is known, and has nothing to do with this.

Comment 45

[Mike Pinkerton (not reading bugmail)]

here's what i see. with the text "|foopy foopy foopy|" (no quotes), on 98,
::GlobalSize returns 44 bytes. On NT, it returns 40, which is correct.

Why would 98 be different? If it were an alignment thing, then why 44 of all
things? That isn't a multiple of 8, where 40 is!

Rod? Any ideas? I need help from a win32 guru on this one big time.

Comment 46

[James Everingham]

GlobalAlloc does guarantee allignment on an 8 byte boundary on windows.  Im not
sure why you are seeing a 4 byte discrepincy, but I would imagine that since it
is one 32 bit pointer size less than 8, it might be using it for some heap
pointer storage or something.  The resultant memory block is not guaranteed to
be divisible by 8, just alligned on an 8 byte boundary and at least as large as
you requested.  Is someone possibly using the returned allocated block size
instead of the requested block size?

Comment 47

[Mike Pinkerton (not reading bugmail)]

got a fix, but need someone to test on 98 for me. i'll check it in when the tree
opens.

Comment 48

[Mike Pinkerton (not reading bugmail)]

fix checked in (again).

Comment 49

[Mike Pinkerton (not reading bugmail)]

removing status since this should be fixed, but it still needs testing on 98.

Comment 50

[Katsuhiko Momoi]

I have Win98-Japanese and have access Win98-US. But neither has a debugger on it,
will these do?

Comment 51

[Mike Pinkerton (not reading bugmail)]

yeah. if you get extra garbage characters when you do a paste, it ain't fixed ;)

Comment 52

[Katsuhiko Momoi]

** Checked with 12/8/99 Win32 Mozilla build **

We don't have today's Win32 commercial build yet but the results
from the Mozilla build indicates that there are still some
problems left. Here are the problems I see under Win 98 Japanese:

1. When copying a JPN string from Editor to Browser Location field
   or Mail subject field.
   we get something like:

    <body>JPN_string</body>1/8Note

where JPN-string = the original copy material in Japanese, and
1/8Note = a musicall symbol indicating 1/8th note.

If I copy an ASCII string, I get something like the following:

    <body>JPN_string</body>Sbox

    where Sbox = a small blank square box symbol

2. When copying a string from Mail Subject field to
   the Mail "To: " field, I get a whole bunch of other HTML
   related things included.

   <body style="font-family: -moz-fixed; background-color: rgb(255,255,255);
white-space: pre; ">test string</body>SBox

The above results are all based on copying from HTML editor
field, if it is from a plain text mail Edit field to other fields,
I get worse results in that the pasted materials look all like
what I describe in #2 above.

I'll wait for the commercial build but we probably should re-open
this bug in any case.

Comment 53

[Mike Pinkerton (not reading bugmail)]

well crap.

Comment 54

[Mike Pinkerton (not reading bugmail)]

can i say how much i hate windows?

Comment 55

[Mike Pinkerton (not reading bugmail)]

does anyone have a win98 machine on which i can debug this? or are you seeing
this on NT as well?

Comment 56

[Akkana Peck]

The body (and other html) tags are a regression in today's build: that's bug
21208 and I'm looking at it.  You'll probably need a fix for that bug before
being able to debug this one meaningfully. :-(

Comment 57

[Mike Pinkerton (not reading bugmail)]

no, i don't think so. it's the extra gunk at the end that's all i care about. i
could care less about what is being passed in.

Comment 58

[Katsuhiko Momoi]

I'm seeing pretty much the same problem with today's build on
Win NT4-US. The musical 1/8th note is visible if you copy from
an Edit field into a location field, Mail "To:" field or
"Subject" field among others.

Comment 59

[Mike Pinkerton (not reading bugmail)]

ah, ok. i wasn't seeing it with data i pasted from _outside_ the app, so maybe
i'll take another look with data copied from w/in the app.

thanks for the pointers.

Comment 60

[Mike Pinkerton (not reading bugmail)]

Ok, here's the scoop now:

when data is copied from an outside app, all is ok.

when data is copied from an ender text area, it is given to the clipboard with a
CR/LF on the end. The clipboard preseves the line endings given to it since it
doesn't know how to interpret the data. When the data is pasted, the CR/LF is
converted into an LF (correctly) and the data is passed back to the ender text
area handling the paste command.

The extra "garbage" is simply the linefeed character. This isn't a clipboard bug
as far as i can tell, it's just that ender text widgets can't handle being given
text that ends in a linefeed.

reassigning to Akkana

Comment 61

[Mike Pinkerton (not reading bugmail)]

i should have been more specific, i think this has to do with single-line text
fields not being able to handle the LF. obviously multi-line ender widgets like
composer can handle it.

I also have some clipboard code that needs to be checked in. I wasn't totally
innocent. If you want, Akkana, I can mail you my diff if i can't get it checked
in before you look at this.

I just checked and i do not see this behavior (the extra character) in composer,
only single-line text widgets.

Comment 62

[Katsuhiko Momoi]

How about Mail Plain Text Edit field? It takes multiple lines
of course but has the same problem. So if you have HTML Editor
string like the following:

XXXXXXX
YYYZZZZ

and you copy from the beginning up to the end of Y, then paste
that into Mail Composer field for Plain Text, you get somethng like
this:

<body>XXXXXX<br>YYY</body>1/8Note

  where 1/8Note = the 1/8 musical note

I also have a quesiton, why should it be copied to the clipboard with
CR/LF when the end of Y seuqence as above is not the end of the line?
I had thought that CR/LF is copied if you selected the string to
inlcude, say, the beginning of the next line but not when the selection
area does not extend to the next line?

Comment 63

[Akkana Peck]

Yes, please send me your clipboard diffs if you haven't already checked them
in.  I'll look at the output sink and figure out why it's appending linebreak
characters.  There used to be some code there to explicitly prevent that from
happening, so that may have gotten broken somehow.

Comment 64

[Mike Pinkerton (not reading bugmail)]

i'm ready to check in my patch when i get approval from chofmann.

I think that i never really did fix the problem with cr/lf on win32 after looking
at my code last night. my patch will do that once and for all. That would explain
why you still are seeing the extra character, even in multi-line text fields.
Sorry for the confusion.

it's copied to the clipboard that way because that's how it's given to me ;)
maybe that is a _separate_ bug for akkana.

Comment 65

[Akkana Peck]

Yes indeed, there's a terminal newline on copied data.  I'm fairly sure this is
a recent regression.  I'm hot on the trail ...

Comment 66

[Akkana Peck]

Fix checked in.

Comment 67

[Jerry Baker]

I tested it on Win98 Se with 1999120919. The test is really doing odd things
with paste now. It only inserted odd characters at the end of the string once,
but I can't reproduce it yet. It does put line feeds in odd places and makes the
ENTER key behave oddly though.

If I open a mail composition window and type the word "this", SHIFT+HOME to
select it and CTRL+C to copy it, then press enter and CTRL+V to paste it on the
next line - the composition window inserts a line feeds before the string. If I
then press ENTER (remember that the cursor is at the end of the word "this"
after pasting it), then the pasted line is moved down to the line below it. This
makes it on a line two lines below the original string and inserts the cursor on
the line below the original string. It's very odd and difficult to describe in
writing. If you play with CTRL+V and ENTER you will see it.

Comment 68

[rhp (gone)]

Not sure if this is related, but there is a bug that I fixed, but lots of
people are running into with the release builds because I don't think my
changes were picked up in those builds.

It is a problem with garbage at the very end of a message. If you are seeing
weirdness at the end of quotations, you want to make sure that your
mozilla\mailnews\mime\src directory is up to date and try it again.

- rhp

Comment 69

[Akkana Peck]

Jerry, can you try this with a 12/10 build?  My fix for this bug wouldn't have
been in a 12/09 build since I didn't check it in 'til that afternoon.

Comment 70

[Katsuhiko Momoi]

I've just looked at Win98 SE-Japanese and intra-Mozilla
copy/pasting using JPN strings. The results look very good.
All the problems I described in my 1999-12-08 16:40 notes
seem to be gone.
We'll now look at this using Latin 1 and other lang strings
and see if it's OK.

Comment 71

[Katsuhiko Momoi]

The build I used is: 12109908 Win32.

Comment 72

[marina]

Using Latin-1 data on 1999-12-09 Win build on my NT40 machine the original
problem is gone. Display looks correct.

Comment 73

[Mike Pinkerton (not reading bugmail)]

yessssssssss!!! ;)

Comment 74

[ji]

It works fine on Linux 1999121008-M12 build.

Comment 75

[marina]

with today 1999-12-13-08-M12 build inthe end of the plain text and html
attachment i'm getting random numbers. I think of reopening.

Comment 76

[Katsuhiko Momoi]

If this is just a display bug, could it be the problem dealt with in
Bug 21314? This bug is for the copy/paste operation.

Comment 77

[Mike Pinkerton (not reading bugmail)]

did you copy/paste, or are you just seeing random text at the end of messages?
This bug is only for copy/paste bugs, not just any random garbage anywhere in the
UI.

Comment 78

[Katsuhiko Momoi]

** Re-checked with 12/14/99 Win32 & MacOS builds **

I've been using intra-Mozilla copy/pasting for a few days
on Win and Mac platforms without seeing the extraneous
characters as I copy from an Text area to widgets like
Mail Subject field, Navigator Location field, etc.
On Linux, too, we have had a report that the extraneous character
is gone.

I think it is now safe to conclude that this has been fixed.
Marking the fix verified.