99048 - [RFE] Automatically mark newsgroup read [when leaving folder]

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[Antti Boman]

Outlook Express lets you choose an option to automatically mark a newsgroup read
when you switch to another group or folder. I don't know how many people like
this, but at least I've found it very useful (instead of manual action: context
menu or Ctrl+Shift+C) when I want to browse multiple newsgroups quickly.

Comment 1

[Ashley Bischoff (blog at handcoding.com)]

No dupes found. Confirming RFE.

Comment 2

[Gilles Durys]

maybe bug 59263

Comment 3

[Ashley Bischoff (blog at handcoding.com)]

Gilles: bug 59263 does seem somewhat related, but not quite a dupe, since the
reporter wants similar functionality along with auto-mark-read.

Comment 4

[Håkan Waara]

What's this doing in the Backend?

-> Mailnews FE

Comment 5

[Antti Boman]

When I reported about this one, the bug form didn't have "Mail Window Front End"
in the drop-down menu. So, as the system told me to do, I guessed.

Comment 6

[[e-mail]]

Didn't 4.x have "Next [Unread] Group" that did this?

Comment 7

[Antti Boman]

Might be, but that's different from my original request, which doesn't require
you to jump to the next group. A kind of trigger that runs when you leave a
newsgroup, whether it's another newsgroup or folder you go to or exit the
program itself.

Comment 8

[Gilles Durys]

Neil : that is bug 59263

Comment 9

[Karsten Düsterloh]

*** Bug 227824 has been marked as a duplicate of this bug. ***

Comment 10

[Nik the Greek]

An improvement over Outlook Express' behavior would disable the
auto-marking-as-read of a newsgroup if, say, shift is pressed when leaving the
group (or live bookmark for that matter). 
Every once in a while you're not finished reading the new items and you still
want to switch groups or close the application, and this makes sure you won't
miss any unread items (especially when one uses the 'only unread' view).

Comment 11

[Karsten Düsterloh]

[e-mail] aber hob zu reden an und schrieb:
> An improvement over Outlook Express' behavior would disable the 
> auto-marking-as-read of a newsgroup if, say, shift is pressed when
> leaving the group (or live bookmark for that matter).

Such a mechanism would be nice, but I don't think that the key-holding would be
a good idea, especially if we get multiple folder selection sometime...
We should keep the topic of this bug as is, at least for now.

Well, I have a kind of solution running in my tree that does following:
whenever a folder change is notified, the (new hidden) integer bitmask pref
"mailnews.mark_message_read.allmask" will be read. It may have several bits set,
for different types of folders (imap, mail, news, etc.). If allowed by the pref,
the folder's messages will all be marked as read.

Taking, btw. ;-)

Comment 12

[Karsten Düsterloh]

Attachment: Mark all messages read on leaving folder

This patch uses these bits:

const MAIL_MARK_FOLDER_READ_NEWS = 0x0001;  // news:
const MAIL_MARK_FOLDER_READ_MBOX = 0x0002;  // mailbox:
const MAIL_MARK_FOLDER_READ_IMAP = 0x0004;  // imap:

I.e. user_pref("mailnews.mark_message_read.allmask", 6); will mark all messages
as read when leaving the folder in imap and 'normal' mailboxes, but not for
newsgroups. Since the pref normally isn't defined, the entire feature will be
turned off by default.

Comment 13

[Karsten Düsterloh]

Comment on attachment 159968 [details] [diff] [review]
Mark all messages read on leaving folder

This is a much requested feature by Outlook (Express) converts - David, what do
you think?
I'd make a patch for TB also, if you request. ;-)

Comment 14

[David :Bienvenu]

Comment on attachment 159968 [details] [diff] [review]
Mark all messages read on leaving folder

looks good to me - to make this really useful, we'd probably want to add yet
some more prefs ui for it.

Comment 15

[David :Bienvenu]

and sure, thx, a patch for tbird would be good.

Comment 16

[[e-mail]]

Comment on attachment 159968 [details] [diff] [review]
Mark all messages read on leaving folder

This will go wrong when you have multiple windows open, because all the
observers will see all the topic changes - although this can be worked around I
suggest mail:setupToolbarItems isn't an ideal way to hook into the change of
folder (presumably you ported an extension), try calling a function from
FolderPaneSelectionChange and OnUnloadMessenger instead.

I'm also not sure why you'd want to distinguish between imap and mailbox.

Comment 17

[Karsten Düsterloh]

Attachment: cleaner version

Addressed Neil's comments:
> try calling a function from FolderPaneSelectionChange and
> OnUnloadMessenger instead.

Done. Saves even some code. ;-)

> I'm also not sure why you'd want to distinguish between imap and mailbox.

One may want to keep the unread state for IMAP (online) mail, but lose it for
local mail that can even be searched offline.

Comment 18

[[e-mail]]

Comment on attachment 160316 [details] [diff] [review]
cleaner version

>+function MarkAllReadOnLeavingFolder(aFolder)
Maybe call this OnLeavingFolder in case someone wants to add to it in future?

>+    folderMask = gPrefBranch.getIntPref("mailnews.mark_message_read.allmask");
>+  }
>+  catch(e){/* ignore */}
>+  if (folderMask && folderURI &&
>+      ((/^news:/   .test(folderURI) && (folderMask & MAIL_MARK_FOLDER_READ_NEWS)) ||
>+       (/^mailbox:/.test(folderURI) && (folderMask & MAIL_MARK_FOLDER_READ_MBOX)) ||
>+       (/^imap:/   .test(folderURI) && (folderMask & MAIL_MARK_FOLDER_READ_IMAP))))
Hmm... you could extract the scheme from the URI and read a boolean pref for
that scheme e.g. user_pref("mailnews.mark_message_read.news", true); which
would automatically scale to other URI types (RSS?).

Comment 19

[Daniel Wiermans]

Could this alos be done for the Thunderbird client?

Comment 20

[Karsten Düsterloh]

Attachment: even more simplistic beauty

Addressed Neil's comment #18.

Comment 21

[Karsten Düsterloh]

Uh, forgot the renaming, but that can be done when checking in, I guess. ;-)

Comment 22

[[e-mail]]

Comment on attachment 169647 [details] [diff] [review]
even more simplistic beauty

>+    const scheme = aFolder.Value.match(/^[^:]+/)[0].toLowerCase();
Beauty? :-P For a start you shouldn't need .toLowerCase, then you should
probably use .URI rather than .Value in case bsmedberg ever gets around to
fixing the resource factory. Except you might find either aFolder.server.type
or aFolder.server.localStoreType will suffice. I think the latter is equivalent
to your regexp although it treats rss and pop3 as local folders; .type will of
course be "none" for local folders.

>+    markAllRead = gPrefBranch.getBoolPref("mailnews.mark_message_read." + scheme);
>+  }
>+  catch(e){/* ignore */}
>+  
>+  if (markAllRead)
>+  {
>+    // mark all messages of aFolder as read
>+    goDoCommand('cmd_markAllRead');
>+  }
No point having this outside the try block.

Comment 23

[Karsten Düsterloh]

Attachment: shortest version yet

> Beauty? :-P

Beauty lies within the eye of the beholder. ;-)
This patch is getting smaller and simpler with each incarnation - that's nearly
a kind of /mathematical/ beauty! :)

> For a start you shouldn't need .toLowerCase, then you should
> probably use .URI rather than .Value in case bsmedberg ever gets around to
> fixing the resource factory. Except you might find either aFolder.server.type

> or aFolder.server.localStoreType will suffice. I think the latter is
equivalent
> to your regexp although it treats rss and pop3 as local folders; .type will
of
> course be "none" for local folders.

Since aFolder.server.type differs between rss and pop3, I'll take that.

> No point having this outside the try block.

Ah, even more simplicity ahead. ;-)

Comment 24

[[e-mail]]

Comment on attachment 169662 [details] [diff] [review]
shortest version yet

I think that's as simplified as it's going to get ;-)

Comment 25

[Karsten Düsterloh]

Attachment: same as attachment 169662, but for TB (against /mail trunk)

David, this is exactly the same patch as for Seamonkey, but against the current
TB trunk. If you, too, are happy with it, please feel free to check both in. :)

Comment 26

[Karsten Düsterloh]

Asking for 1.8a6, since David already gave r+ for a much more complicated
solution...

Comment 27

[Asa Dotzler [:asa]]

we're not going to hold the release for this. setting the blocking- flag.

Comment 28

[Karsten Düsterloh]

Comment on attachment 169662 [details] [diff] [review]
shortest version yet

carrying over David's r+

Comment 29

[Karsten Düsterloh]

Comment on attachment 169669 [details] [diff] [review]
same as attachment 169662 [details] [diff] [review], but for TB (against /mail trunk)

carrying over Neil's sr+ (if possible)

Comment 30

[Asa Dotzler [:asa]]

I'm concerned about adding new features this late in the cycle. What kind of
testing has been done on this? What's the risks of taking it?

Comment 31

[Karsten Düsterloh]

I've running it in my (debug) tree since, I tested it with some nightlies and
applied the patch to an official TB 1.0 build; all on Windows.
I'd say the risk is minimal (because it's just two function calls and a new
function that is entirely guarded by a try-catch-block), but that's not for me
to decide...

Comment 32

[Asa Dotzler [:asa]]

Comment on attachment 169662 [details] [diff] [review]
shortest version yet

a=asa for checkin to 1.8a6

Comment 33

[Asa Dotzler [:asa]]

Comment on attachment 169669 [details] [diff] [review]
same as attachment 169662 [details] [diff] [review], but for TB (against /mail trunk)

a=asa for checkin to 1.8a6

Comment 34

[Karsten Düsterloh]

Both patches (attachment #169662 [details] [diff] [review] and attachment #169669 [details] [diff] [review]) have been checked in,
but at least the Seamonkey nightly 2005-01-10-07 shows some need for a
supplementary edge case patch. The default use without any of the hidden prefs
introduced here (and hence 1.8a6) is not affected.

The problem is this, when having turned on the automatic marking for e.g. a pop3
account:

Case 1:
- select a subfolder of the pop3 account that contains unread mails
- now select the pop3 account entry
=> In OnLeavingFolder, gDBView is still pointing to the old folder that is of
interest for us. But the command controller called already uses the new
selection to check the command's availability, thus denying to mark the messages
as read...

Case 2: similar to case 1, but the other way around
- select the pop3 account entry
- now select a subfolder of the pop3 account
=> The command controller has |gDBView == null| (we left an account folder
without data), but the command is enabled. Thus we fail in the command handler
when calling gDBView functions...
<http://lxr.mozilla.org/mozilla/source/mailnews/base/resources/content/mail3PaneWindowCommands.js#610>

Supplementary patch possibilities:
1) Calling gDBView.doCommand in directly OnLeavingFolder, without bothering
about isCommandEnabled, but checking gDBView.
2) Moving the gDBView.doCommand call into a new functions that will be called by
 both OnLeavingFolder and doCommand.

I'll provide a patch for #2.

Comment 35

[[e-mail]]

Actually I'd prefer #1.

Comment 36

[Karsten Düsterloh]

Attachment: Supplementary patch (Seamonkey)

Comment 37

[[e-mail]]

Comment on attachment 170966 [details] [diff] [review]
Supplementary patch (Seamonkey)

gDBView will never exist for a server, so you can get rid of the isServer test;
also you might as well test for gDBView first. Otherwise sr=me.

Comment 38

[Karsten Düsterloh]

Attachment: Supplementary patch (Thunderbird)

Same as attachment.cgi 170966, but with Neil's comments addressed. See comment
#34 why this supplementary patch is needed.

Comment 39

[Karsten Düsterloh]

Attachment: Supplementary patch (Seamonkey) [checkin version]

Carrying over David's r+ and Neil's sr+.

Comment 40

[Christian :Biesinger (don't email me, ping me on IRC)]

supplementary patches checked in:
Checking in mail/base/content/commandglue.js;
/cvsroot/mozilla/mail/base/content/commandglue.js,v  <--  commandglue.js
new revision: 1.50; previous revision: 1.49
done
Checking in mailnews/base/resources/content/commandglue.js;
/cvsroot/mozilla/mailnews/base/resources/content/commandglue.js,v  <-- 
commandglue.js
new revision: 1.253; previous revision: 1.252
done

Comment 41

[Karsten Düsterloh]

And, lo!, it's finally done...

Comment 42

[Mike Cowperthwaite]

Trying this patch out with Moz 1.8b-0120.  Creating, and setting true, the pref
  mailnews.mark_message_read.news
doesn't have any affect, but the pref
  mailnews.mark_message_read.nntp
does work as expected.  Altho, after trying it, I'll never use this.  :)

Comment 43

[Karsten Düsterloh]

Working types are currently |nntp|, |pop3|, |imap|, |rss| (in TB) and |none|
(for Local Folders).

Comment 44

[Rinaldi J. Montessi]

Appears we have a regression in the past couple of  days.  The 200707110202 nightly and my own checkout finish: Thu Jul 12 00:58:34 EDT 2007 are not triggering the "mark all read" event on leaving the group.  "mailnews.mark_message_read.nntp ;true" is set in prefs.js.

Comment 45

[Karsten Düsterloh]

Please file new bugs for new issues.