494811 - collapsed-by-default is the wrong default when actions apply to collapsed threads

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[David Ascher (:davida)]

in particular, we don't want the collapsed-thread experience to interfere with normal behavior, e.g. reading a bunch of mail that just came in, which currently is collapsed by default.

collapsing should be an explicit action (left arrow/click-on-twisty).

Comment 1

[David :Bienvenu]

My initial thought is that we should just expand threads with new messages in a threaded view, not that we should expand all threads by default. Or we could expand threads with unread messages in them. What were you/Bryan thinking should be the normal behavior?

Comment 2

[David Ascher (:davida)]

I think the ideal is hard to get -- philor was heard saying that the current model (collapsed by default, actions-on-collapsed-threads) means that there's a rough transition for the "bugmail triage" use case -- collapsing by default means that people who want to work through message by message have to a) know about right-arrow-to-expand, and then b) have an extra keystroke per message.

So, it feels that the collapsed default works well for getting rid of whole threads, but if you're reading mostly threads where you want to process every message, then collapsed threads isn't ideal.

Some possibilities:

 - use the folder summary pane (not yet landed) to explain about the right-arrow-to-expand shortcut

 - we could (ugh) consider what mail.app does, which is to have an extra row in the threadpane correpsonding to the thread, whether collapsed or not.  I suspect that would be a fair bit of work in the C++ view code.

 - nothing else springs to mind

bryan might have better ideas...

Comment 3

[Mark Banner (:standard8)]

I agree its a difficult transition from the bugmail triage use case.

> a) know about right-arrow-to-expand

I learn something new every day...

>  - nothing else springs to mind

My only thought was a conversation view on the thread. Then you can just scroll through reading the thread and decide to archive or do some other action. Not sure we'd do that in the TB 3 time frame though.

Comment 4

[David Ascher (:davida)]

> My only thought was a conversation view on the thread. 

I like the idea  There are a couple of tricky things about that, based on the exptoolbar conversation view experiments :

a) figuring out how to stream the message bodies into something that can be displayed safely in the html iframe is full of trickiness, especially w.r.t. html email (not a problem for bugmail, obviously, but actually in the case of bugmail, there are other issues like in-flow rendering of ascii art).  Right now I stream enough to generate snippets, and it seems to work well for that, but I don't think we want to take on full body, inline attachments, etc.

b) the current summary view is really meant for triage, with archive/delete the only two 'promoted' operations.  a real conversation view would naturally lead to reply, reply-inline, which is a whole other ball-o-wax, which we most definitely can't do for tb3.

I'll think about it some more, but other thoughts welcome.

Comment 5

[David :Bienvenu]

I gots no problem getting rid of whole threads because I can click on the little thread widget to select the whole thread, and then press delete or click the delete toolbar button. But some may not have my dexterity with the mouse :-)

If we did auto expand of threads with new or unread, then we have the opposite problem of the default collapsed threads, which is how to operate on the whole thread, but one or two left-arrows gets the user into the state where the root msg is selected in the collapsed state. But there's a highly annoying focus issue where the arrow keys only work if the thread pane has the focus.

I still think auto expanding threads with new feels like the right way to deal with triage. I think the summary view is more meant for dealing with the thread as a whole, which isn't just for triage of newer messages - in fact, for me it's more for whatever you call dealing with older messages/threads.

Comment 6

[Phil Ringnalda (:philor)]

If "expand threads with new messages" means "expand them when a message is added, and leave them expanded until manually collapsed" then that would work perfectly for my use-case (the word "new" makes me nervous what with the temporary nature of "new mail in this folder", thus the qualification).

Also? Huh, right-arrow expands a thread! (Though my problem comes from not noticing it's a thread, not from not being able to expand it.)

Comment 7

[David :Bienvenu]

(In reply to comment #6)
> If "expand threads with new messages" means "expand them when a message is
> added, and leave them expanded until manually collapsed" then that would work
> perfectly for my use-case (the word "new" makes me nervous what with the
> temporary nature of "new mail in this folder", thus the qualification).

There are two cases - one is when mail arrives into an open folder, and the other is when new mail arrives in a folder while its closed. The former is easy to handle, and I have a small patch that does that. But the latter is trickier, since when we're building up the view, we'd need to know which threads to expand, and we'd have to choose between threads with new and threads with unread. It all depends on whether you keep your unread count under control, I guess. I think I'd be OK with new, and instead of leaving a folder to do something else, I'd use a new tab, so I wouldn't lose the new/expand state in the original folder.

Comment 8

[David :Bienvenu]

Attachment: expand thread when new msg comes into view - checked in.

davida, here's a patch to try out.

Comment 9

[David :Bienvenu]

Comment on attachment 381801 [details] [diff] [review]
expand thread when new msg comes into view - checked in.

this is mostly whitespace changes - the last little bit does the work.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 381801 [details] [diff] [review]
expand thread when new msg comes into view - checked in.

Yep, let's give this a try. r/sr=Standard8

Comment 11

[David :Bienvenu]

Comment on attachment 381801 [details] [diff] [review]
expand thread when new msg comes into view - checked in.

I've landed the patch, but I'll leave this open to decide what we want to do, if anything, about new messages that arrive when a folder is not open.

Comment 12

[David :Bienvenu]

this does not seem to always handle the case where we have a single message in a thread, and a new message gets added to the view+thread, perhaps because we don't think the single message thread is collapsed, so we don't try to expand it.

Comment 13

[[e-mail]]

Um, I just found this bug yesterday after some search on this new behaviour.

For years there was this strange behaviour that some thread was expanded when opening a folder. That was annoying, but this now ... Let me put it this way: I can't stand this for the future.

Please, please make it at least disengageable.

Comment 14

[David :Bienvenu]

(In reply to comment #12)
> this does not seem to always handle the case where we have a single message in
> a thread, and a new message gets added to the view+thread, perhaps because we
> don't think the single message thread is collapsed, so we don't try to expand
> it.

I looked into this a bit more - even though we're not expanding the thread, we often end up moving it, which forces selectionChanged to get called, which means we call summarizeSelection. Even though summarizeSelection realizes that it should show a thread summary, it fails w/o an error (probably an exception getting thrown and dropped). So even if I were to fix the view code to notice when the selected message has become a collapsed thread (in the case where the thread doesn't move), and call selection changed, it doesn't seem like it would help.

Comment 15

[Otto Koehn]

I extremely dislike it, when threads are expanded by default. Make it at least disengageable in the very near future!

Comment 16

[Alexander Kluss]

The problem with the expand of threads by default is that the new function override the pref "mailnews.scroll_to_new_message". When this pref was false then are all threads are collapsed.

Comment 17

[Alexander Kluss]

Could everyone fix this patch that the threads are colapsed if the pref "mailnews.scroll_to_new_message" is set to false? The current experience is very ugly. It'S very unpleasent that all threads with new postings are expanded if I open a news groups. It's too long for me that it need 30 to 45 seceonds to open a news group with threads with more than 500 postings. And this at a PC that is up to date.

Comment 18

[Hartmut Figge]

(In reply to comment #17)
> The current experience is very ugly.

You are too polite. This patch is a disaster. Fortunately i can backout it for my own builds, but not everyone can do so.

Comment 19

[David :Bienvenu]

I know davida isn't very happy with the way this works either - we could back this out for b3 and try again during b4...

Comment 20

[David :Bienvenu]

Attachment: patch that backs out just the expand new threads part

since I'm on vacation, I'm going to let Dan decide if he wants to do this for b3 or not...I'm not backing out the formatting cleanups in the original patch, which is why this patch is smaller.

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

OK, after some discussion with davida, the new theory is to make it so that autoexpanding _only_ happens if you're viewing the thread that received the new message; all other threads will not be autoexpanded.

Comment 22

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: autoexpand only the current thread if it gets a new message, rather than all threads

Here's a patch that, for the single message case expands the thread, and, for all other cases forces selection to recomputed, which in turn causes any thread summaries to be updated.  

Should be able to do this without the icky shotgun approach, but I thought I'd start by getting this version in the bug.

Comment 23

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: partial autoexpand, v2

OK, this does better.  It only calls SelectionChanged if there's a high likelihood that it's going to do something: in the case where a selection summary that contains the changed thread is currently being displayed.

Comment 24

[Alexander Kluss]

(In reply to comment #22)

> Created an attachment (id=388625) [details]
> autoexpand only the current thread if it gets a new message, rather than all
> threads

If there an option that I can deactivate this absurd function?

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 388640 [details] [diff] [review]
partial autoexpand, v2

Mark, I'm going to take you up on your offer of reviewing and landing this before the morning builds.  We'll want a post facto ui-r from Bryan after he gets back as well.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

As Andrew pointed out, it would be a fine thing to have a test with this, but I'm too tired to write one now, and I think it's more important that we get this fix for beta 3 at the cost of having less-than-perfect test coverage.  Adding in-testsuite? so that doesn't get forgotten.

Comment 27

[Mark Banner (:standard8)]

(In reply to comment #24)
> (In reply to comment #22)
> 
> > Created an attachment (id=388625) [details] [details]
> > autoexpand only the current thread if it gets a new message, rather than all
> > threads
> 
> If there an option that I can deactivate this absurd function?

I had a similar reaction initially however now I've had it explained it isn't absurd. I think it is worth explaining so let me try to explain with some cases:

- If you have selected a single message (no thread) and you receive a response to that thread, you don't want to suddenly switch onto the thread summary view whilst you're reading the message, therefore we must auto-expand that thread.

- If you have selected a message thread that is collapsed, you are viewing the summary, and then it receives a new message, you just want to view the updated summary, so don't expand.

- If you have selected a message in a thread that is expanded and a new message comes in, leave it as expanded.

- If you have selected a message in a different thread to the one where a new message is received, leave both threads in their current collapsed/expanded states.

Comment 28

[Andrew Sutherland [:asuth] (he/him)]

Attachment: improved autoexpand v3. has unit test and passes it.

More evidence in support of unit tests ;)

Comment 29

[Mark Banner (:standard8)]

Comment on attachment 388667 [details] [diff] [review]
improved autoexpand v3.  has unit test and passes it.

>+        // if this message is new, the thread is collapsed, it is the
>+        // root and it was displayed, expand it so that the user does
>+        // not find that their message has magically turned into a summary

nit: Capitial I on the if and a full stop at the end please.

This seems to be doing what I expect. We should still get ui-review on it though when Bryan gets back.

Comment 30

[Andrew Sutherland [:asuth] (he/him)]

pushed: http://hg.mozilla.org/comm-central/rev/1c016412c990

I'm leaving it up to dmose/davida/bienvenu/clarkbw to figure out when to mark fixed.

Comment 31

[Dan Mosedale (:dmosedale, :dmose)]

Oooh, nice further optimization.  And muchas gracias for the unit test changes!  Going to resolve now; the ui-review? will still show up on clarkbw's review page.

Comment 32

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

Comment on attachment 388667 [details] [diff] [review]
improved autoexpand v3.  has unit test and passes it.

yep, thanks mark for the use case / break down.

Comment 33

[Sum Yung Guy]

I would like to request to re-open this bug. The fix was good, except I would like to ask for a modification in one use-case. The (somewhat zen-like) question is: what is the collapsed/expanded state of a thread with only one message?

I agree it should work as summarized in comment 27. My problem is with a sub-case in the fourth case, where a thread should be left in its existing state if a new message comes in when it is not being viewed. The fix assumes that a single-message thread is collapsed; i.e., when a reply comes to a single message, a collapsed thread is created. I would suggest that a single-message thread should be assumed to be expanded, and should be created as expanded.

Like most people, I keep currently-active threads expanded so I can quickly see and read new messages coming in. Since the user is not given any control over the collapsed/expanded state of a single-message thread, it makes the most sense that brand-new threads should be created expanded, and then give me the option to collapse them when they're not active or important.

It's particularly a problem for me because I've always bcc'd sent messages to my inbox, rather than using the "sent" folder. When someone answers an e-mail, I can't see or read it without first expanding the newly-created collapsed thread.

I do see how this behavior might be a problem (as several highly-annoyed german guys have pointed out here :-) in a newsgroup or mailing list where a lot of new, long, unread threads show up at once and you want to scan through to see which ones you want to read or not. But I'd think for the great majority of people reading personal mail, the default should be for new threads to be created expanded. Perhaps it could be a per-folder property, or at least an about:config preference?

Thanks...