341266 - Unread Folders should refresh

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[Mark Richards]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060611 Thunderbird/3.0a1 - Build ID: 2006061105

In this version of Thunderbird, which I am seriously in love with, the "unread folders" feature does not refresh automatically (nor is there any facility to command it).  The result is that, once a message is read, the folder remains in the sidebar window.

Either a refresh feature on a timer, or a refresh event on each message read, would be useful here.

--

Enhancement: message list in sidebar hyperlinks to an action only to the extent of the title.  i.e.: if the title is a short word, "Notice", clicking to the right of "notice" on the same line does not produce the action to display the email.  Extending the clickable action area to the right margin of the sidebar would be preferable, along with alternating background color shading for the mail list.


Reproducible: Always

Comment 1

[Mike Cowperthwaite]

(In reply to comment #0)
> Either a refresh feature on a timer, or a refresh event on each message read,
> would be useful here.

This, actually, is an enhancement request.  Note the list does update when new mail arrives in an unlisted folder.

Generally speaking, the sorts of updates you're looking for aren't implemented 
-- for instance, if you have a MailView of Unread messages, reading one of 
those messages doesn't take it out of the list.  Otherwise, as soon as you read the message, the message is removed from the list, and so the selection changes and you end up reading some other message.


> Enhancement: message list in sidebar hyperlinks to an action only to the
> extent of the title.  i.e.: if the title is a short word, "Notice", clicking
> to the right of "notice" on the same line does not produce the action to
> display the email.  Extending the clickable action area to the right margin
> of the sidebar would be preferable, along with alternating background color
> shading for the mail list.

This is bug 340867, and is a small regression.

Notes on terminology:  What you're calling "Sidebar" is the "Folder Pane"; the 3pane window doesn't have a sidebar.  The items listed in the folder pane are not "messages," they're folders (or newsgroups); and they're not "hyperlinks."

Comment 2

[Mark Richards]

>>if you have a MailView of Unread messages, reading one of 
>>those messages doesn't take it out of the list.  Otherwise, as soon as you read
>>the message, the message is removed from the list, and so the selection changes
>>and you end up reading some other message.

Good point.  However I think the behaviour in the case of the Folder List (thanks for correcting my use of terms) is special, and different.  The context is not a list of messages but a folder which, in the case of listing these filtered as "unread", is a placeholder for a function.

Once the function is achieved, i.e., messages have been read, it seems logical that the folder should then go away.  

Side note, I am trying to find the best place to pose questions about this trunk, specifically having to do with themes and extensions.  I don't know where the conversations on this trunk are being held, except here, of course.

Comment 4

[David :Bienvenu]

The folder can only go away once the user selects a different folder (i.e., we can't have the selected folder go away).

I think this is probably a reasonable thing to do, and only slightly tricky.

Comment 5

[bugzilla]

As originator of bug 379516 (marked as dup of this one in comment #3) I'd like to add my support for this solution. :-)

Comment 6

[Philippp]

Perhaps it can change to gray, instead of vanishing instantly?
I would suggest its a good idea to indicate *some* change, because otherwise the user thinks it is broken.

Comment 8

[u88384]

IMO, bug 689141 (which I reported) is not exactly a duplicate - but it's similar (and seemingly equally unimportant).

Comment 9

[l0f4r0]

I support this feature request.
And yes, indeed, in my opinion the read folder should disappear from the folder pane once the user selects another folder.

Comment 11

[prempa]

In my opinion, in the view "Unread folders", once the emails inside a single folder are all read, that folder should disappear from the pane.
I think it's called "unread folders" for a reason.

Comment 12

[Rick]

(In reply to prempa from comment #11)

In my opinion, in the view "Unread folders", once the emails inside a single folder are all read, that folder should disappear from the pane.
I think it's called "unread folders" for a reason.

Agreed

Comment 13

[adamb]

This feature not working as it should.
I do have multiple mail accounts, thus the list is growing when mail will arrive.
I am now required to manually switch the "compact view" mode to hide all what's been read.
An improvement in it to automatically refresh and hide is necessary.

Comment 14

[Vform]

In my opinion, in the view "Unread folders", once the emails inside a single folder are all read, that folder should disappear from the pane.
I think it's called "unread folders" for a reason.
+1

In the 18 years of this ticket I probably disabled this view a few times because it didn't make too much sense that way, especially since I use hibernate and Thunderbird usually stays open for weeks. Now I'm trying it again with the workaround from adamb by switching back an forth between the compact view (that removes old "read unread" folders). But I think I would prefer a fading of "read unread" folders after some minutes.

Comment 15

[github]

(In reply to Vform from comment #14)
But I think I would prefer a fading of "read unread" folders after some minutes.

That would be really great
+1

Comment 17

[adamb]

But I think I would prefer a fading of "read unread" folders after some minutes.

To address this there should be a global in-background periodic refresh, and user should be able to set refreshing time for his needs.

Comment 19

[porretta.danilo]

To address this there should be a global in-background periodic refresh, and user should be able to set refreshing time for his needs.

This is not necessarily true, you just need to check every time you read an email from a folder with unread messages

Comment 20

[Simon Arlott]

Attachment: Remove folders from the Unread view when they have no unread mail (patch for v133.0)

I've made a patch for this that appears to work but there may be some unexpected cases where it doesn't remove a folder when it could do. It has to wait until the newly-read folder is deselected otherwise removing it from the unread list that it's selected in would immediately close it as soon as you read the last email and cause another folder to be selected instead.

Ideally I'd prefer not to have the UI change immediately because it can be disruptive when the visible folder tree moves around, but that's currently what happens when new unread mail arrives.

I have to check the ancestors and descendants before removing a folder otherwise it's possible to get an ancestor folder to be removed from the tree by using "Mark Folder Read" on a descendant. Similarly, if any descendants are selected then their ancestors can't be removed.

I assume the performance impact of calling MailServices.folderLookup.getFolderForURL() on all unread folders will be low but it would be possible to have a huge quantity of unread folders in a collapsed tree.

Comment 21

[adamb]

(In reply to Simon Arlott from comment #20)

I've made a patch for this that appears to work
Im on Beta v134.0.b4, but doesn't have it, what's the fastest way to have this feature? Nightly?

Comment 22

[Hartmut Welpmann [:welpy-cw]]

Simon, regarding the usual workflow for fixing a bug, please see https://developer.thunderbird.net/thunderbird-development/getting-started and https://developer.thunderbird.net/thunderbird-development/fixing-a-bug.

Comment 23

[Dave Miller [:justdave]]

Comment on attachment 9445784 [details] [diff] [review]
Remove folders from the Unread view when they have no unread mail (patch for v133.0)

I am not a Thunderbird developer. You need someone who works on Thunderbird to review this.

Comment 24

[Geoff Lankow (:darktrojan) [OOO, back 7 Apr]]

Simon, this looks promising. Thank you!

There's a few things to fix that need the build environment to be set up:

• This change almost certainly breaks the test mail/base/test/browser/browser_folderTreeQuirks.js (and possibly others). If it doesn't, it should. So that would need to be fixed.
• The linter needs to run on the file you changed. (You could fix the problems manually but it's much easier to have the tools do it for you.)
• We no longer accept patches by Bugzilla attachment, only via Phabricator. The "fixing a bug" link in comment 22 has the details.

You wouldn't need to do a full compile for a patch like this, it's suitable for an artifact build. The worst part about getting set up is downloading the mozilla-central repository, which can take some time.

If you think you might contribute more patches in the future, it would be worth the one-time effort of getting set up. Or if you're just here to scratch an itch, one of us could take over from here.

Comment 25

[Alessandro Castellani [:aleca]]

Echoing what Geoff said.
Let us know if you have time to continue working on this patch and pass through Phabricator, or if you prefer to hand it over to one of the core devs.
Thank you so much for your contribution.

Comment 26

[Ben Merritt]

I started on my own patch for this (before discovering that there's already an in-progress patch, naturally). If it makes sense for me to take over on polishing things up, I'd be happy to help. If someone else should move this forward instead, I can at least post my patch for comparison in case there's anything useful.

So far, my patch is pretty similar to Simon's except that (1) I'm using folderTree.selectedRow instead of gFolder (not quite sure which should be preferred), I'm using object identity comparison on the DOM nodes rather than comparing folder URIs (so the folder isn't considered "selected" for the sake of this functionality if it's selected in another view), and I haven't implemented removal on selection change (which doesn't look like it should be too hard based on Simon's work).

Comment 27

[Ben Merritt]

Also, it looks like https://bugzilla.mozilla.org/show_bug.cgi?id=432310 may be a duplicate of this bug.

Comment 28

[Magnus Melin [:mkmelin]]

Simon, are you still working on this? See comment 24.

Comment 29

[Simon Arlott]

I've spent enough time on this patch already, I don't have enough knowledge of Mercurial or time to jump through numerous hoops to use Phabricator and I'm not familiar enough with the code to modify the tests. My changes have been working well for me so far, although I don't do unusual folder selection changes.

Comment 30

[Magnus Melin [:mkmelin]]

Ben, do you want to take over?

Comment 31

[Ben Merritt]

I'd be happy to. I've had a couple more ideas on how to handle some edge cases, and I've started filing off the rough edges on the tests. It still may take me a bit to get everything polished up, but I'll keep chipping at it until things are in decent shape.

Comment 32

[Ben Merritt]

Attachment: Bug 341266 - Remove read folders from Unread Folders view. r=#thunderbird-reviewers

Comment 33

[Ben Merritt]

Attachment: WIP: Bug 341266 - Follow-up: Add more context to folder tests

Comment 34

[Ben Merritt]

Attachment: WIP: Bug 341266 - Follow-up: Restrict automatic removal of selected folders.

Comment 35

[Ben Merritt]

Attachment: WIP: Bug 341266 - Follow-up: Clean up unread folders on selection change.

Comment 36

[bugzilla]

This "feature request" annoyed me a lot and I finally found a workaround and thought maybe others might be interested in it also - https://www.reddit.com/r/Thunderbird/comments/yycqct/comment/iyqf62b/?utm_source=share&utm_medium=web2x&context=3

Comment 37

[Ben Merritt]

I think I've got the basic logic fully working in my patch, but I'm rather stuck on the tests. I've posted in the Matrix room but haven't gotten a response on that yet. If that stays stalled out, I'll probably have to break out the good old printf debugger. (The JS debugger doesn't help a lot with the events that are involved here, and I haven't worked out a proper C++ debugging environment yet.)

Comment 38

[Simon Arlott]

My patch was originally developed on v133, was working ok on v115, but ever since upgrading to v128 I've experienced problems changing to the next unread folder (and other folders) after the current unread folder becomes fully read. Clicking the next unread folder doesn't do anything. I have to select the current folder again before it starts working. The same problem happens if I change to Ben's patch.

e.g.

1. Start with Unread Folders and All Folders visible
2. Make one folder have an unread email
3. Select this folder in the Unread Folders list
4. Mark the email as read
5. Select the Inbox from the All Folders list
6. Select the Drafts folder from the All Folders list (nothing happens)
7. Select the Sent folder from the All Folders list (changes to Sent folder)
8. Select the Drafts folder from the All Folders list (now it changes to the Drafts folder)

Comment 40

[Ben Merritt]

I'm still making the occasional effort to move this forward. It looks like something (specifics still unknown) in a JavaScript event handler is causing a call to nsMsgDBFolder::SetMsgDatabase(nullptr), which drops the event handlers that are listening to changes in the folder's read status. The event listeners are eventually reinstated, but by then, the test has already moved on.

Comment 41

[Hartmut Welpmann [:welpy-cw]]

(In reply to Ben Merritt from comment #40)

I'm still making the occasional effort to move this forward. It looks like something (specifics still unknown) in a JavaScript event handler is causing a call to nsMsgDBFolder::SetMsgDatabase(nullptr), which drops the event handlers that are listening to changes in the folder's read status. The event listeners are eventually reinstated, but by then, the test has already moved on.

When a folder is selected, the database for the previously shown folder generally is closed, so marking the message header read doesn't notify about a change in the folder. Instead use the markAllMessagesRead or markMessagesRead for the folder.

Some more hints: D248684 needs to be rebased, and while you are at it, I'd really recommend combining all four patches into one (using hg histedit -> fold). If there are only some minor issues you are stuck with, but the patch is generally working, it's mostly better to just request review, otherwise any questions you have may not get the attention they deserve.

After all, your contribution already seems to work just fine!

Comment 42

[Ben Merritt]

I've got a rebased version locally; I just haven't pushed it up yet since I've been hacking on it locally.

The granular splitting of the patches is mostly an artifact of the typical commit style at my day job and my own need to keep things organized while I was sorting out my own mental model of the code; if reviewers would generally prefer a single larger patch, I can fold those down.

Comment 43

[Ben Merritt]

Attachment: WIP: Bug 341266 - WIP: expand selection tests

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b418552d0dab
Remove read folders from Unread Folders view. r=darktrojan

Comment 45

[Toby Pilling [:tobyp]]

@Ben Merritt you rock! Thanks so much for your help and persistence with this!