If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Thunderbird 3.0b3

Status

Thunderbird
Folder and Message Lists
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Magnus Melin, Assigned: dmose)

Tracking

({regression})

Trunk
Thunderbird 3.0b3
x86
Linux
regression
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [needs ui-r clarkbw][patch landed])

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
Flags: blocking-thunderbird3+

Comment 1

8 years ago
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.
(Assignee)

Comment 2

8 years ago
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...
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b4
(Reporter)

Comment 3

8 years ago
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.
(Reporter)

Comment 4

8 years ago
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.
Blocks: 474701
Summary: delete all in thread snaps to last msg → imap mark as deleted: delete all in thread snaps to last msg of thread pane
(Reporter)

Updated

8 years ago
Duplicate of this bug: 500677
(Assignee)

Comment 6

8 years ago
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?
Target Milestone: Thunderbird 3.0b4 → Thunderbird 3.0b3

Updated

8 years ago
Duplicate of this bug: 502525
(Assignee)

Updated

8 years ago
Assignee: nobody → dmose
(Assignee)

Updated

8 years ago
Whiteboard: [needs patch, review]
(Assignee)

Comment 8

8 years ago
Created attachment 388133 [details] [diff] [review]
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.
Attachment #388133 - Flags: ui-review?(clarkbw)
Attachment #388133 - Flags: superreview?(bienvenu)
Attachment #388133 - Flags: review?(bienvenu)
(Assignee)

Updated

8 years ago
Summary: imap mark as deleted: delete all in thread snaps to last msg of thread pane → imap mark as deleted: delete multiple messages snaps to last msg of thread pane
(Assignee)

Updated

8 years ago
Whiteboard: [needs patch, review] → [needs ui-review clarkbw, review bienvenu]
(Assignee)

Updated

8 years ago
Whiteboard: [needs ui-review clarkbw, review bienvenu] → [has patch, needs ui-r clarkbw, r+sr bienvenu]
(Assignee)

Updated

8 years ago
Attachment #388133 - Flags: superreview?(bugzilla)
Attachment #388133 - Flags: superreview?(bienvenu)
Attachment #388133 - Flags: review?(bugzilla)
Attachment #388133 - Flags: review?(bienvenu)
(Assignee)

Updated

8 years ago
Whiteboard: [has patch, needs ui-r clarkbw, r+sr bienvenu] → [needs r+sr Standard8, ui-r clarkbw]
Attachment #388133 - Flags: superreview?(bugzilla)
Attachment #388133 - Flags: superreview+
Attachment #388133 - Flags: review?(bugzilla)
Attachment #388133 - Flags: review+
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
Whiteboard: [needs r+sr Standard8, ui-r clarkbw] → [needs ui-r clarkbw][patch landed]
(Assignee)

Comment 10

8 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 11

8 years ago
sorry I didn't get to this until today ... I'll clean up the K&R braces later as penance :-)
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.
Attachment #388133 - Flags: ui-review?(clarkbw) → ui-review+
You need to log in before you can comment on or make changes to this bug.