76888 - line wrapping problems in plain text mail compose

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P1)

Description

[Christopher Blizzard (:blizzard)]

Build is on Apr 20, 2001.

Sometimes when composing a mail message with the plain text editor there are
some serious line wrapping problems.  It always happens when I'm typing a line
and I hit delete to delete the character at the end of the line that I'm typing
on.  What happens is that the line is suddenly indented one or two characters
and whitespace in between the current sentence and the previous is eaten.  If
you back the cursor up to the beginning of the line before the indentation and
try to backspace over them to correct them it thinks that it's at the beginning
of the line and it actually backs up to the previous line but the line itself is
still indented one character.

I still don't have a 100% reproducable test case yet.

Comment 1

[Christopher Blizzard (:blizzard)]

OK, I have a reproducable test case for this!  Shaver has reproduced it so it's
not just me.  Please note that neither of us have signature files which may be
signifigant.

Steps to reproduce:

o Bring up a mail compose window

o Click on the content area to bring focus to it

o Type the following.  Special keys are in brackets.

foo.
<return>
<return>
--Chris
<up arrow>
<up arrow>
<space>
hh
<backspace>

Comment 2

[varada]

Shouldnt this belong to the editor team? akkana? jfrancis?

Comment 3

[Joe Francis]

arg!  ancient crusty code must die.  i have a wonderful fix for this: delete 
everything inside of nsTextEditRules::DidDeleteSelection().

Comment 4

[Christopher Blizzard (:blizzard)]

Nope, that didn't fix it.

Comment 5

[Joe Francis]

it does for me.  But i have other patches in the tree I was testing with.  
However, with these other patches, I can duplicate the bug, and then see it fixed 
when I remove DidDeleteSelection code.  

Let's see what happens later on after ws work lands (65557).  Then we will all be 
on same playing field.

Comment 6

[Mike Shaver (:shaver emeritus)]

God, how I wish we could fix this for 0.9.  Sending plaintext mail _sucks_
without this fix.

Comment 7

[Christopher Blizzard (:blizzard)]

Yeah, I want this for 0.9 too.  Sending mail does suck right now with this. 
It's biting me all over the place and it looks just plain awful. ( No pun
intended. )

Comment 8

[Dimitrios]

I was able to reproduce Christopher Blizzard's test case on build 2001042012.
This is a problem on all operating systems.

Comment 9

[Joe Francis]

Attachment: editor diffs

Comment 10

[Joe Francis]

the attached diffs fix the testcase.  shaver/blizzard please give it a go.  i 
invite interested parties to lobby for 0.9 landing if they like the results.

adding mozilla0.9 keyword

Comment 11

[Christopher Blizzard (:blizzard)]

Yeah, they fix the test case here too.  Let me do some testing through the day
to make sure I don't see it again and I don't see any other nasty regressions...

Comment 12

[Joe Francis]

assigning to me; cc'ing kathy

Comment 13

[Chris Waterson]

want want want!

Comment 14

[Christopher Blizzard (:blizzard)]

OK, I haven't seen any problems in plain text all day.  Feels baked to me.  Is
this ready for r/sr?

Comment 15

[Joe Francis]

I'm ready for r/sr/approval assuming none of my testers have any worries.

Comment 16

[Joe Francis]

i snag has been found.  
Taking out the code that did adjacent textnode merging has exposed two problems
in textareas/textfields:

1) layout will not properly wrap text that is split across text nodes (it treats
the node boundary as a wrap point)
2) the editor chokes on zero-length text nodes in some cases

I believe I can write a nsTextEditRules::WillDeleteSelection() function to hide
the first problem and fix the second.

i'll update the patch when ready.

Comment 17

[Joe Francis]

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

Comment 18

[Joe Francis]

improved (?) patch.  I now scan around for empty text nodes and squish them.

Comment 19

[Joe Francis]

Attachment: patch to editor/base

Comment 20

[Asa Dotzler [:asa]]

bringing TM back to 0.9 to keep this in line for review/approval, etc. 

Comment 21

[Brendan Eich [:brendan]]

>1) layout will not properly wrap text that is split across text nodes (it
>   treats the node boundary as a wrap point)

Is there a layout bug on file for this?  If so, please cite it here; if not,
please file it.

/be

Comment 22

[Joe Francis]

testing looks good so far.

>1) layout will not properly wrap text that is split across text nodes (it
>   treats the node boundary as a wrap point)

is bug http://bugzilla.mozilla.org/show_bug.cgi?id=77394

It requires empty text nodes around to happen.

Comment 23

[Joe Francis]

testing looks good.  I'm ready to land as soon as reviews are in.

Comment 24

[Simon Fraser [no longer active]]

r=sfraser

Comment 25

[kinmoz]

sr=[e-mail]

Comment 26

[Joe Francis]

ok, my understanding is that I land on trunk & 0.9 branch independently right 
after branch is cut (and after hyatts checkins clear, which should be before 
branch is cut).  If anyone objects to that plan you better speak up.

Comment 27

[Christopher Blizzard (:blizzard)]

The 0.9 branch has been cut so you should be OK to check in there.  I have oone
checked out already.  Let me know if you want me to check this in for you.

Comment 28

[Joe Francis]

i have landed this on branch.  trunk landing will occur if we open the trunk 
before 5pm today.

Comment 29

[Joe Francis]

landed on trunk

Comment 30

[selmer (gone)]

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

Comment 31

[esther]

Using trunk build 2002-03-1 on winxp 20020304 on linux on mac OS 9.1 20020304,
and the original scenario this is fixed.  Verified