41336 - Can't delete vestige of empty mail quote

Status: VERIFIED FIXED

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

Description

[Akkana Peck]

Reply to any plaintext mail message using the html compose window.

Click somewhere in the middle of the message, and hit return, and type something.

Now suppose you want to delete the rest of the quoted material (the usual case
for me, since the end of a message is probably a signature or multiply-quoted
stuff or something).  Mouse down somewhere near the beginning of the last quoted
block and sweep to somewhere a little past the end of the last quoted line, so
that all the text you want to delete is selected.

Hit backspace.

Most of the time (but not always -- if you go way down below the quoted material
this doesn't seem to happen) the text will go away but you'll get one or more
"blue bars" indicating that there's still a blockquote there.

Output HTML indicates that these blue bars look like this:
<blockquote type="cite" cite="[e-mail]">
<pre wrap>

</pre>
</blockquote>
i.e. they contain a pre with one or more newlines inside.

Further attempts to remove these blue bars can be maddening.  Sometimes delete-
to-end (^K on Unix) deletes them, but usually not.  Sometimes backspace deletes
them.  Sometimes backspace doesn't delete them, but moves the visible caret back
to the end of the previous line (the line you typed), but further operations
seem confused about which of those two lines the caret is on.  Once I got it to
crash when I backspaced when positioned on the blue bar (this was after several
attempts to kill it with ^K) but I haven't been able to reproduce that to get a
stack trace, alas.

I suggest that the rule should be that if we are splitting a cite blockquote and
the part after the split contains only whitespace (including only whitespace
inside a pre), we should not split the blockquote.

Comment 1

[Joe Francis]

accepting

Comment 2

[rubydoo123]

moving to m17

Comment 3

[Joe Francis]

nominating for b3

Comment 4

[rubydoo123]

Adding correctness and nsbeta3+

Comment 5

[rubydoo123]

added correctness and nsbeta3+

Comment 6

[rubydoo123]

setting priority in status whiteboard - medium

Comment 7

[rubydoo123]

m18

Comment 8

[rubydoo123]

setting to future and adding helpwanted

Need to triage bugs to meet the glidepath for pr3

Comment 9

[Joe Francis]

moz 0.9

Comment 10

[Joe Francis]

moving a bunch of 0.9 bugs to 0.9.1

Comment 11

[Joe Francis]

096

Comment 12

[Joe Francis]

fixed by patch to bug 99523

Comment 13

[Joe Francis]

fixed by 99523 checkin

Comment 14

[sujay]

akkana, does this work for you now? thanks.

Comment 15

[Akkana Peck]

Nope, more or less following my original instructions (actually, I brought up an
empty html mail compose window, selected some plaintext from this bug and
shift-middleclicked to get some quoted stuff into the compose window, then
followed instructions from that point), when I hit backspace, the caret warps to
the end of the previous line (I expected a new line to be created, but end of
the previous line isn't too terrible) but I still see a blue bar a few lines
below the caret.

Mailcompose doesn't have a debug menu and sending doesn't seem to work from this
build, so I can't tell you whether the blockquote would actually be sent, but
the blue bar is still there visually and I still can't delete it.

Sujay, do you not see the blue bar?

Comment 16

[Akkana Peck]

Turns out it did send even though it told me it didn't.  The undeletable
blockquote after the final caret is indeed sent:
[various legit quoted stuff]
  </blockquote>
asdfjaksdlfh<br>
asdf
  <blockquote type="cite">
    <pre></pre>
    </blockquote>
    </body>
    </html>

Comment 17

[Joe Francis]

Yeah, I still see this too.  I'll try to figure out what's going on for 099. 
Sleep is for the weak!

Comment 18

[Joe Francis]

Attachment: patch for editor/libeditor/html/nsHTMLEditRules.cpp

The inablity to easily delete the piece of mailcite is tricky.	But that whole
issue can be saved for a sunny day since this patch fixes the original sin:
allowing mailcite splitting to create empty pieces of cites.

Comment 19

[kinmoz]

Comment on attachment 69439 [details] [diff] [review]
patch for editor/libeditor/html/nsHTMLEditRules.cpp

sr=[e-mail]

Comment 20

[Akkana Peck]

Sorry, but I'm still seeing it even with the patch.  One difference (I think
it's different, anyway) is that after the mailcite splits, the second cite
(after the new text inserted) seems to have a blank line at the beginning of it
(maybe that was there before the patch and I just didn't notice).

But I can still get a single line's worth of blue bar after my typed text, which
I can't mouse or downarrow to and therefore can't delete.

Comment 21

[Akkana Peck]

A little more info:
When I split the mailcite, the half after my split seems to start with a blank
line.  If I mouse down at the beginning of the first non-blank line, sweep to
the end, and delete, I'm left with a blank mailcite which I can move the caret
to; I can't ^K delete it (that's a problem),  but I can backspace-delete it (so
it's not as big a problem as it was originally).   But if I mouse down near the
bogus empty line after the split, and sweep to the end, and hit backspace, I'm
left with a one-line phantom mailcite which can't be moused or arrowed to and
hence can't be deleted.

Comment 22

[kinmoz]

Strange the patch seems to work ok for me. I can delete the 2nd half of a split 
quote just fine. Akk even walked me through her example on IRC and I could not 
reproduce what she was seeing.

I can only get the blank line Akk mentioned above, if I hit return at the end of 
a quoted line. So it looks like it might be due to the fact that the 2nd half of 
the split gets the <br> of that line since the caret was before it? 

Comment 23

[Joe Francis]

Hmm.  i tested both selecting to the end of the quote *and* putting the caret at
the end of a quote that had a trailing br, and both had worked fine for me.  

Ah, perhaps it's because Akkana is hitting backspace after selecting.  I was
hitting return.  I guess i'll have to put a similar workaround in the deletion
code.  Some blocks we leave around when delete everything in them, on the theory
that you are getting rid of the contents of the block, not the block itself. 
And then you can just hit backspace again in the empty block to get it to go
away.  But mailcites are a bit differrent: I guess if someone selects all of the
contents and hits backspace (or even types) we should get rid of the cite.

I'll post a new patch soon that does this.

Comment 24

[Joe Francis]

btw, Akana, why would you expect ^K to delete the mail cite if yo uhad selection
collpased in it?  You would have to select across it for that to work.

Comment 25

[Joe Francis]

Attachment: mega patch to libeditor

including mega patch to libeditor that has a bunch of fixes.  this is so it
will apply.  the only change to the relavant earlier patch for this bug is that
RemoveEmptyNodes() now allows mailcites to be deleted when empty, and is in
fact very aggressive about deleting them (it will delete them even if they
contain a single br).  

Akkana can you give this a try?

Comment 26

[Joe Francis]

hmm, maybe i should make this more sophisticated?  When deleting a mailcite that
has a br, I should probably keep the br.

Comment 27

[Akkana Peck]

^K deletes from the current position to the end of the line, unless it's already
on the end of a line, in which case it deletes the line.

Since there was nothing (except for the mailcite) on the empty line, I would
expect ^K to delete it; that's what it does if I place the caret on any other
blank quoted line.

Comment 28

[Joe Francis]

fixed on tip

Comment 29

[sujay]

Akkana, is this working for you now? thanks...I tried following
your steps and I don't see any problem...if you can double check

Comment 30

[Akkana Peck]

Yes!  It's not perfect, but it never leaves me stuck with quotes I can't delete.

Here are the nits I would pick (minor):
1. After I've selected all of the last quoted block, if I hit return, it goes
away, but if I hit backspace, I'm left inside an empty quoted block.  I can hit
backspace again and the quote will go away, but I would have expected it go go
away with the first backspace.
2. In a debug build, when I hit the second backspace to delete the quoted block,
I get an assertion:
###!!! ASSERTION: nsRange::PopRanges() got error from SetEnd():
'NS_SUCCEEDED(res)', file nsRange.cpp, line 1034

I'll mark this bug verified since the bad part of the user-visible behavior is
gone.  But Joe, should I open a new bug on the assertion?