Can't delete vestige of empty mail quote

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
Editor
P3
normal
VERIFIED FIXED
18 years ago
16 years ago

People

(Reporter: Akkana Peck, Assigned: Joe Francis)

Tracking

({helpwanted})

Trunk
mozilla0.9.9
x86
Linux
helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

18 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="39381E25.6080209@netscape.com">
<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.
(Assignee)

Comment 1

18 years ago
accepting

Status: NEW → ASSIGNED
Target Milestone: --- → M16

Comment 2

18 years ago
moving to m17
Target Milestone: M16 → M17
(Assignee)

Comment 3

18 years ago
nominating for b3
Keywords: nsbeta3

Comment 4

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

Updated

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

Comment 5

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

Comment 6

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

Comment 7

18 years ago
m18
Target Milestone: M17 → M18

Comment 8

18 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

Updated

18 years ago
Keywords: mailtrack
(Assignee)

Comment 9

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

Updated

18 years ago
Keywords: mailtrack
(Assignee)

Comment 10

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

Updated

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

Updated

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

Updated

17 years ago
Whiteboard: [behavior]
Target Milestone: mozilla0.9.2 → mozilla1.0
(Assignee)

Comment 11

17 years ago
096
Target Milestone: mozilla1.0 → mozilla0.9.6
(Assignee)

Updated

17 years ago
Whiteboard: [behavior] → EDITORBASE; 4 days; [behavior]
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.7
(Assignee)

Comment 12

17 years ago
fixed by patch to bug 99523
(Assignee)

Updated

17 years ago
Whiteboard: EDITORBASE; 4 days; [behavior] → EDITORBASE; fixinhand; patch in 99523
(Assignee)

Comment 13

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

Comment 14

16 years ago
akkana, does this work for you now? thanks.
(Reporter)

Comment 15

16 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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 16

16 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>
asdfjaksdlfh<br>
asdf
  <blockquote type="cite">
    <pre></pre>
    </blockquote>
    </body>
    </html>
(Assignee)

Comment 17

16 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

Updated

16 years ago
Keywords: nsbeta1+
Whiteboard: EDITORBASE → EDITORBASE+
(Assignee)

Comment 18

16 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.
(Assignee)

Updated

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

Comment 19

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

sr=kin@netscape.com
Attachment #69439 - Flags: superreview+

Updated

16 years ago
Whiteboard: EDITORBASE+; fixinhand; need r=, sr=, testng → EDITORBASE+; fixinhand; need r=, testng
(Reporter)

Comment 20

16 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.
(Reporter)

Comment 21

16 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

16 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? 
(Assignee)

Comment 23

16 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.
(Assignee)

Comment 24

16 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.
(Assignee)

Comment 25

16 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
(Assignee)

Updated

16 years ago
Whiteboard: EDITORBASE+; fixinhand; need r=, testng → EDITORBASE+; fixinhand; need r=, sr=, testng
(Assignee)

Comment 26

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

Comment 27

16 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.
(Assignee)

Comment 28

16 years ago
fixed on tip
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → FIXED

Comment 29

16 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
(Reporter)

Comment 30

16 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?
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.