Closed
Bug 99048
Opened 23 years ago
Closed 20 years ago
[RFE] Automatically mark newsgroup read [when leaving folder]
Categories
(SeaMonkey :: MailNews: Message Display, enhancement)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: antti.boman, Assigned: mnyromyr)
References
Details
Attachments
(4 files, 4 obsolete files)
2.64 KB,
patch
|
neil
:
superreview+
asa
:
approval1.8a6+
|
Details | Diff | Splinter Review |
2.70 KB,
patch
|
Bienvenu
:
review+
asa
:
approval1.8a6+
|
Details | Diff | Splinter Review |
1.09 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
1.11 KB,
patch
|
mnyromyr
:
review+
mnyromyr
:
superreview+
|
Details | Diff | Splinter Review |
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•23 years ago
|
Summary: Automatically mark newsgroup read → [RFE] Automatically mark newsgroup read
Comment 1•23 years ago
|
||
No dupes found. Confirming RFE.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•23 years ago
|
||
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•23 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•23 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•23 years ago
|
||
Didn't 4.x have "Next [Unread] Group" that did this?
Reporter | ||
Comment 7•23 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.
Assignee | ||
Updated•20 years ago
|
Summary: [RFE] Automatically mark newsgroup read → [RFE] Automatically mark newsgroup read [when leaving folder]
Assignee | ||
Comment 9•20 years ago
|
||
*** Bug 227824 has been marked as a duplicate of this bug. ***
Comment 10•20 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•20 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•20 years ago
|
Assignee: sspitzer → mnyromyr
Status: ASSIGNED → NEW
Assignee | ||
Comment 12•20 years ago
|
||
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•20 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•20 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•20 years ago
|
||
and sure, thx, a patch for tbird would be good.
Comment 16•20 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•20 years ago
|
||
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•20 years ago
|
Attachment #160316 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160316 -
Flags: review?(bienvenu)
Comment 18•20 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?).
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 19•20 years ago
|
||
Could this alos be done for the Thunderbird client?
Assignee | ||
Updated•20 years ago
|
Attachment #160316 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160316 -
Flags: review?(bienvenu)
Assignee | ||
Comment 20•20 years ago
|
||
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•20 years ago
|
||
Uh, forgot the renaming, but that can be done when checking in, I guess. ;-)
Comment 22•20 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•20 years ago
|
||
> 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•20 years ago
|
Attachment #169647 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #169647 -
Flags: review?(bienvenu)
Comment 24•20 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•20 years ago
|
||
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•20 years ago
|
||
Asking for 1.8a6, since David already gave r+ for a much more complicated solution...
Flags: blocking1.8a6?
Comment 27•20 years ago
|
||
we're not going to hold the release for this. setting the blocking- flag.
Flags: blocking1.8a6? → blocking1.8a6-
Updated•20 years ago
|
Attachment #169669 -
Flags: review?(bienvenu) → review+
Assignee | ||
Comment 28•20 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•20 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•20 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•20 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•20 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•20 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•20 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•20 years ago
|
||
Actually I'd prefer #1.
Assignee | ||
Comment 36•20 years ago
|
||
Attachment #170966 -
Flags: superreview?(neil.parkwaycc.co.uk)
Comment 37•20 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•20 years ago
|
||
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•20 years ago
|
Attachment #171050 -
Flags: review?(bienvenu) → review+
Assignee | ||
Comment 39•20 years ago
|
||
Carrying over David's r+ and Neil's sr+.
Attachment #170966 -
Attachment is obsolete: true
Attachment #171051 -
Flags: superreview+
Attachment #171051 -
Flags: review+
Comment 40•20 years ago
|
||
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•20 years ago
|
||
And, lo!, it's finally done...
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 42•20 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•20 years ago
|
||
Working types are currently |nntp|, |pop3|, |imap|, |rss| (in TB) and |none| (for Local Folders).
Comment 44•17 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•17 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.
Description
•