Last Comment Bug 99048 - [RFE] Automatically mark newsgroup read [when leaving folder]
: [RFE] Automatically mark newsgroup read [when leaving folder]
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- enhancement with 4 votes (vote)
: ---
Assigned To: Karsten Düsterloh
: esther
Mentors:
: 227824 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2001-09-10 04:43 PDT by Antti Boman
Modified: 2007-07-12 11:14 PDT (History)
9 users (show)
asa: blocking1.8a6-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Mark all messages read on leaving folder (4.34 KB, patch)
2004-09-24 06:31 PDT, Karsten Düsterloh
mozilla: review+
neil: superreview-
Details | Diff | Review
cleaner version (2.69 KB, patch)
2004-09-28 01:51 PDT, Karsten Düsterloh
no flags Details | Diff | Review
even more simplistic beauty (2.78 KB, patch)
2004-12-27 11:24 PST, Karsten Düsterloh
no flags Details | Diff | Review
shortest version yet (2.64 KB, patch)
2004-12-27 15:11 PST, Karsten Düsterloh
neil: superreview+
asa: approval1.8a6+
Details | Diff | Review
same as attachment 169662, but for TB (against /mail trunk) (2.70 KB, patch)
2004-12-27 16:46 PST, Karsten Düsterloh
mozilla: review+
asa: approval1.8a6+
Details | Diff | Review
Supplementary patch (Seamonkey) (1.13 KB, patch)
2005-01-11 14:28 PST, Karsten Düsterloh
neil: superreview+
Details | Diff | Review
Supplementary patch (Thunderbird) (1.09 KB, patch)
2005-01-12 09:08 PST, Karsten Düsterloh
mozilla: review+
Details | Diff | Review
Supplementary patch (Seamonkey) [checkin version] (1.11 KB, patch)
2005-01-12 09:26 PST, Karsten Düsterloh
mnyromyr: review+
mnyromyr: superreview+
Details | Diff | Review

Description Antti Boman 2001-09-10 04:43:14 PDT
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) 2001-09-10 07:02:13 PDT
No dupes found. Confirming RFE.
Comment 2 Gilles Durys 2001-09-10 07:35:22 PDT
maybe bug 59263
Comment 3 Ashley Bischoff (blog at handcoding.com) 2001-09-10 08:17:28 PDT
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 2001-09-10 08:53:05 PDT
What's this doing in the Backend?

-> Mailnews FE
Comment 5 Antti Boman 2001-09-10 23:34:37 PDT
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 neil@parkwaycc.co.uk 2001-09-14 04:32:43 PDT
Didn't 4.x have "Next [Unread] Group" that did this?
Comment 7 Antti Boman 2001-09-14 04:53:07 PDT
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 2001-09-14 09:16:40 PDT
Neil : that is bug 59263
Comment 9 Karsten Düsterloh 2004-09-23 07:53:08 PDT
*** Bug 227824 has been marked as a duplicate of this bug. ***
Comment 10 Nik the Greek 2004-09-23 11:19:17 PDT
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 2004-09-24 01:02:24 PDT
bugzilla-daemon@mozilla.org 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 2004-09-24 06:31:21 PDT
Created attachment 159968 [details] [diff] [review]
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 2004-09-24 06:35:49 PDT
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 2004-09-24 07:46:44 PDT
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 2004-09-24 08:08:42 PDT
and sure, thx, a patch for tbird would be good.
Comment 16 neil@parkwaycc.co.uk 2004-09-27 11:56:30 PDT
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 2004-09-28 01:51:18 PDT
Created attachment 160316 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2004-10-08 16:42:50 PDT
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 2004-12-24 09:30:06 PST
Could this alos be done for the Thunderbird client?
Comment 20 Karsten Düsterloh 2004-12-27 11:24:49 PST
Created attachment 169647 [details] [diff] [review]
even more simplistic beauty

Addressed Neil's comment #18.
Comment 21 Karsten Düsterloh 2004-12-27 11:28:47 PST
Uh, forgot the renaming, but that can be done when checking in, I guess. ;-)
Comment 22 neil@parkwaycc.co.uk 2004-12-27 13:53:34 PST
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 2004-12-27 15:11:30 PST
Created attachment 169662 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2004-12-27 15:52:21 PST
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 2004-12-27 16:46:56 PST
Created attachment 169669 [details] [diff] [review]
same as attachment 169662 [details] [diff] [review], 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 2005-01-04 13:49:33 PST
Asking for 1.8a6, since David already gave r+ for a much more complicated
solution...
Comment 27 Asa Dotzler [:asa] 2005-01-05 13:23:18 PST
we're not going to hold the release for this. setting the blocking- flag.
Comment 28 Karsten Düsterloh 2005-01-08 17:53:09 PST
Comment on attachment 169662 [details] [diff] [review]
shortest version yet

carrying over David's r+
Comment 29 Karsten Düsterloh 2005-01-08 17:53:55 PST
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] 2005-01-08 17:59:54 PST
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 2005-01-08 18:52:15 PST
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] 2005-01-09 12:09:01 PST
Comment on attachment 169662 [details] [diff] [review]
shortest version yet

a=asa for checkin to 1.8a6
Comment 33 Asa Dotzler [:asa] 2005-01-09 12:09:08 PST
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 2005-01-11 02:55:57 PST
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 neil@parkwaycc.co.uk 2005-01-11 09:33:57 PST
Actually I'd prefer #1.
Comment 36 Karsten Düsterloh 2005-01-11 14:28:09 PST
Created attachment 170966 [details] [diff] [review]
Supplementary patch (Seamonkey)
Comment 37 neil@parkwaycc.co.uk 2005-01-11 16:51:00 PST
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 2005-01-12 09:08:32 PST
Created attachment 171050 [details] [diff] [review]
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 2005-01-12 09:26:02 PST
Created attachment 171051 [details] [diff] [review]
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) 2005-01-12 11:40:07 PST
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 2005-01-13 13:31:21 PST
And, lo!, it's finally done...
Comment 42 Mike Cowperthwaite 2005-01-21 13:00:58 PST
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 2005-01-21 16:43:23 PST
Working types are currently |nntp|, |pop3|, |imap|, |rss| (in TB) and |none|
(for Local Folders).
Comment 44 Rinaldi J. Montessi 2007-07-12 08:07:59 PDT
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 2007-07-12 11:14:52 PDT
Please file new bugs for new issues.

Note You need to log in before you can comment on or make changes to this bug.