The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

SeaMonkey
MailNews: Message Display
--
enhancement
RESOLVED FIXED
16 years ago
10 years ago

People

(Reporter: Antti Boman, Assigned: Karsten Düsterloh)

Tracking

Bug Flags:
blocking1.8a6 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 4 obsolete attachments)

(Reporter)

Description

16 years ago
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.

Updated

16 years ago
Summary: Automatically mark newsgroup read → [RFE] Automatically mark newsgroup read
No dupes found. Confirming RFE.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

16 years ago
maybe bug 59263
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

16 years ago
What's this doing in the Backend?

-> Mailnews FE
Assignee: mscott → sspitzer
Component: Mail Back End → Mail Window Front End
(Reporter)

Comment 5

16 years ago
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

16 years ago
Didn't 4.x have "Next [Unread] Group" that did this?
(Reporter)

Comment 7

16 years ago
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

16 years ago
Neil : that is bug 59263
(Assignee)

Updated

13 years ago
Summary: [RFE] Automatically mark newsgroup read → [RFE] Automatically mark newsgroup read [when leaving folder]
(Assignee)

Comment 9

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

Comment 10

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

Comment 11

13 years ago
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. ;-)
Status: NEW → ASSIGNED
(Assignee)

Updated

13 years ago
Assignee: sspitzer → mnyromyr
Status: ASSIGNED → NEW
(Assignee)

Comment 12

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

Comment 13

13 years ago
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. ;-)
Attachment #159968 - Flags: review?(bienvenu)

Comment 14

13 years ago
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.
Attachment #159968 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #159968 - Flags: review?(bienvenu)
Attachment #159968 - Flags: review+

Comment 15

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

Comment 16

13 years ago
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.
Attachment #159968 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
(Assignee)

Comment 17

13 years ago
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.
Attachment #159968 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #160316 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160316 - Flags: review?(bienvenu)

Comment 18

13 years ago
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?).
Product: Browser → Seamonkey

Comment 19

12 years ago
Could this alos be done for the Thunderbird client?
(Assignee)

Updated

12 years ago
Attachment #160316 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160316 - Flags: review?(bienvenu)
(Assignee)

Comment 20

12 years ago
Created attachment 169647 [details] [diff] [review]
even more simplistic beauty

Addressed Neil's comment #18.
Attachment #160316 - Attachment is obsolete: true
Attachment #169647 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #169647 - Flags: review?(bienvenu)
(Assignee)

Comment 21

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

Comment 22

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

Comment 23

12 years ago
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. ;-)
Attachment #169647 - Attachment is obsolete: true
Attachment #169662 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Updated

12 years ago
Attachment #169647 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #169647 - Flags: review?(bienvenu)

Comment 24

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

I think that's as simplified as it's going to get ;-)
Attachment #169662 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 25

12 years ago
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. :)
Attachment #169669 - Flags: review?(bienvenu)
(Assignee)

Comment 26

12 years ago
Asking for 1.8a6, since David already gave r+ for a much more complicated
solution...
Flags: blocking1.8a6?

Comment 27

12 years ago
we're not going to hold the release for this. setting the blocking- flag.
Flags: blocking1.8a6? → blocking1.8a6-

Updated

12 years ago
Attachment #169669 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 28

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

carrying over David's r+
Attachment #169662 - Flags: approval1.8a6?
(Assignee)

Comment 29

12 years ago
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)
Attachment #169669 - Flags: approval1.8a6?

Comment 30

12 years ago
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?
(Assignee)

Comment 31

12 years ago
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

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

a=asa for checkin to 1.8a6
Attachment #169662 - Flags: approval1.8a6? → approval1.8a6+

Comment 33

12 years ago
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
Attachment #169669 - Flags: approval1.8a6? → approval1.8a6+
(Assignee)

Comment 34

12 years ago
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

12 years ago
Actually I'd prefer #1.
(Assignee)

Comment 36

12 years ago
Created attachment 170966 [details] [diff] [review]
Supplementary patch (Seamonkey)
Attachment #170966 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 37

12 years ago
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.
Attachment #170966 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 38

12 years ago
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.
Attachment #171050 - Flags: review?(bienvenu)

Updated

12 years ago
Attachment #171050 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 39

12 years ago
Created attachment 171051 [details] [diff] [review]
Supplementary patch (Seamonkey) [checkin version]

Carrying over David's r+ and Neil's sr+.
Attachment #170966 - Attachment is obsolete: true
Attachment #171051 - Flags: superreview+
Attachment #171051 - Flags: review+
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
(Assignee)

Comment 41

12 years ago
And, lo!, it's finally done...
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 42

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

Comment 43

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

Comment 44

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

Comment 45

10 years ago
Please file new bugs for new issues.
You need to log in before you can comment on or make changes to this bug.