501161 - imap mark as deleted: delete multiple messages snaps to last msg of thread pane

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Magnus Melin [:mkmelin]]

In threaded mode, select all of the message in a thread by clicking the threading indicator (left of the first message in the thread). 

Hit delete to delete all the messages in the thread. The delete works, but: the last message in the thread pane is selected as next message, not the next message after the thread - like it use to.

This is very inconvenient for me as I do this frequently, and current behavior is never what I want. I suspect it's from bug 474701, but haven't verified it.

Comment 1

[Frédéric Buclin]

As far as I can tell, this is a dupe of bug 367689 which I filed 2.5 years ago. I'm pretty sure the problem you see has the same root cause as the one described there.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I can't reproduce this at all in the 20090629 OS X nightly build.  Any chance this is Linux only?

Also, how certain are you that this is a regression?  And what's it a regression against?  Given that the functionality of deleting an entire thread didn't exist in 3.0b2, I suspect that most beta users would not perceive any regression at all.

Based on this set of things, I'm going to move the target milestone to 3.0b4, because I can't see us holding 3.0b3 for this as it's currently understood.  Feel free to convince me that that's a mistake...

Comment 3

[Magnus Melin [:mkmelin]]

Oh this isn't bug 367689. This bug is about selecting all in a thread + delete, which will jump to the last message in the whole thread pane.

Oh I'm sure it's a regression, will check the range later. I'm using the imap "mark as deleted" model, so maybe that affects things. As a correction to my initial comment, it used to not change selection at all (it doesn't need to, as the messages are all there, just with a line through.)

Note that this isn't about the new act on collapsed threads functionality, but the normal select-all-of-thread functionality that has existed since forever.

Comment 4

[Magnus Melin [:mkmelin]]

Regression range is 2009-06-12 -> 2009-06-13
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2009-06-12&enddate=2009-06-13+03%3A00 ... indeed bug 474701 landed then.

As the jumping around is really disturbing i do think this should block b3.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Boy, do I wish I had some data on what percentage of our beta users use the IMAP delete model (or even what percentage of our users in general).  Oh well.

Marking as blocking for now, but whether or not we'd really hold for it if it were the last bug depends a bunch on how much effort it is to fix, I think.

Want to take a run at it?

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: selection after imap delete fix, v1

Since we get a list of selected ranges of messages, I've made the code select the first message after the end of the first range of messages that we've deleted.  Alternately, if mail.delete_matches_sort_order is set to true,
it selects the first message before the start of the first range deleted; requesting UI-review on that.  Bug 349762 has info about the history & rationale for that preference.

I'm assuming that we're not going to get tree selection objects with adjacent/overlapping ranges.  This code doesn't attempt to handle those cases, as it would add considerably to code complexity.  It does spit a warning to stderr in debug builds if it detects the adjacent case so that at least it's possible for a developer to notice if that assumption turned out to be wrong.
The failure mode is that it will merrily select one of the deleted messages.

nsMsgDBView::GetMsgToSelectAfterDelete is gradually growing more complex, which I'm not thrilled with, but it's not obvious to me how one would fix that without a bunch of work.

Comment 9

[Mark Banner (:standard8)]

I've pushed this in time for today's nightly (with ui-review pending) so we can get more testing on it:

http://hg.mozilla.org/comm-central/rev/4fc747bc8ad9

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Marking RESOLVED FIXED, as this is now in the tree.  Bryan, when you get back from vacation, a post-facto UI review would be much appreciated, and if the UI needs more work, a new bug would great.

Comment 11

[David :Bienvenu]

sorry I didn't get to this until today ... I'll clean up the K&R braces later as penance :-)

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 388133 [details] [diff] [review]
selection after imap delete fix, v1

a very late ui-review.  this seems like the right choice and I haven't noticed any negative feedback on the change so I'm assuming it is working within peoples expectations.