Can't delete vestige of empty mail quote

VERIFIED FIXED in mozilla0.9.9



19 years ago
17 years ago


(Reporter: akkzilla, Assigned: mozeditor)




Firefox Tracking Flags

(Not tracked)


(Whiteboard: EDITORBASE+; fixinhand; need r=, sr=, testng)


(1 attachment, 1 obsolete attachment)



19 years ago
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="">
<pre wrap>

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

19 years ago

Target Milestone: --- → M16

Comment 2

19 years ago
moving to m17
Target Milestone: M16 → M17

Comment 3

19 years ago
nominating for b3
Keywords: nsbeta3

Comment 4

19 years ago
Adding correctness and nsbeta3+
Keywords: correctness
Whiteboard: nsbeta3+


19 years ago
Whiteboard: nsbeta3+ → {nsbeta3+}

Comment 5

19 years ago
added correctness and nsbeta3+
Whiteboard: {nsbeta3+} → [nsbeta3+]

Comment 6

19 years ago
setting priority in status whiteboard - medium
Whiteboard: [nsbeta3+] → [nsbeta3+][p:3]

Comment 7

19 years ago
Target Milestone: M17 → M18

Comment 8

19 years ago
setting to future and adding helpwanted

Need to triage bugs to meet the glidepath for pr3
Keywords: helpwanted
Whiteboard: [nsbeta3+][p:3] → [nsbeta3-][p:3]
Target Milestone: M18 → Future
Keywords: mailtrack

Comment 9

19 years ago
moz 0.9
Target Milestone: Future → mozilla0.9


18 years ago
Keywords: mailtrack

Comment 10

18 years ago
moving a bunch of 0.9 bugs to 0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1


18 years ago
Target Milestone: mozilla0.9.1 → mozilla0.9.2


18 years ago
Keywords: nsbeta3
Whiteboard: [nsbeta3-][p:3]


18 years ago
Whiteboard: [behavior]
Target Milestone: mozilla0.9.2 → mozilla1.0

Comment 11

18 years ago
Target Milestone: mozilla1.0 → mozilla0.9.6


18 years ago
Whiteboard: [behavior] → EDITORBASE; 4 days; [behavior]


18 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 12

18 years ago
fixed by patch to bug 99523


18 years ago
Whiteboard: EDITORBASE; 4 days; [behavior] → EDITORBASE; fixinhand; patch in 99523

Comment 13

17 years ago
fixed by 99523 checkin
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 14

17 years ago
akkana, does this work for you now? thanks.

Comment 15

17 years ago
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?
Resolution: FIXED → ---

Comment 16

17 years ago
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 type="cite">

Comment 17

17 years ago
Yeah, I still see this too.  I'll try to figure out what's going on for 099. 
Sleep is for the weak!
Whiteboard: EDITORBASE; fixinhand; patch in 99523 → EDITORBASE
Target Milestone: mozilla0.9.7 → mozilla0.9.9


17 years ago
Keywords: nsbeta1+

Comment 18

17 years ago
Created attachment 69439 [details] [diff] [review]
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.


17 years ago
Whiteboard: EDITORBASE+ → EDITORBASE+; fixinhand; need r=, sr=, testng

Comment 19

17 years ago
Comment on attachment 69439 [details] [diff] [review]
patch for editor/libeditor/html/nsHTMLEditRules.cpp
Attachment #69439 - Flags: superreview+


17 years ago
Whiteboard: EDITORBASE+; fixinhand; need r=, sr=, testng → EDITORBASE+; fixinhand; need r=, testng

Comment 20

17 years ago
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

17 years ago
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

17 years ago
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

17 years ago
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

17 years ago
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

17 years ago
Created attachment 69957 [details] [diff] [review]
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?
Attachment #69439 - Attachment is obsolete: true


17 years ago
Whiteboard: EDITORBASE+; fixinhand; need r=, testng → EDITORBASE+; fixinhand; need r=, sr=, testng

Comment 26

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

Comment 27

17 years ago
^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

17 years ago
fixed on tip
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 29

17 years ago
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

17 years ago
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?
You need to log in before you can comment on or make changes to this bug.