Closed Bug 640147 Opened 14 years ago Closed 14 years ago

Port |Bug 542998 - Need option to disable Archiving completely|

Categories

(SeaMonkey :: MailNews: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b3

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

References

Details

Attachments

(1 file, 1 obsolete file)

Thunderbird just added a pref for disabling archiving completely in bug 542998. This pref will be exposed in the Account Manager UI later so we should have it, too. Parts to be ported from files listed in http://hg.mozilla.org/comm-central/rev/0347b42406e6 mail3PaneWindowCommands.js mailWindowOverlay.js messageWindow.js nsContextMenu.js The rest is either stuff we don't have, tests, back-end or shared (like the default pref in mailnews.js).
messageWindow.js: Forget it, we don't have an Archive button. nsContextMenu.js: Our file is called mailContextMenus.js.
Attached patch patch (obsolete) (deleted) — Splinter Review
Note that this slightly changes the logic (in sync with TB): The first message's folder's server's archiveKeepFolderStructure setting is no longer taken into account (see the removed comments).
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #518104 - Flags: superreview?(neil)
Attachment #518104 - Flags: review?(mnyromyr)
Comment on attachment 518104 [details] [diff] [review] patch >- ShowMenuItem("mailContext-archive", showMailItems && oneOrMore); >+ var canArchive = CanArchiveMsg(); >+ ShowMenuItem("mailContext-archive", showMailItems && oneOrMore && canArchive); No need for the variable. >+ * Checks if the selected messages can be archived. This depends on what folder >+ * they're in and whether archiving is enabled for that identity. s/what/which/ (I think) >+ return gFolderDisplay.selectedMessages.every(function(msg) { >+ return GetIdentityForHeader(msg).archiveEnabled; Permission granted to make it one long line or move the { on its own line. ;-) Activating this now will mean that all archiving will cease to function, because archiveEnabled will be false in old profiles? What is the migration path?
(In reply to comment #3) > No need for the variable. Sure; I was just looking at the context, but in fact even had it inlined at one point in time locally. > >+ * Checks if the selected messages can be archived. This depends on what folder > >+ * they're in and whether archiving is enabled for that identity. > > s/what/which/ (I think) I'd say "what" refers to the folder type (as in "what kind of" -> Archive) rather than a certain folder ("which" -> "Archive"). But I won't get into a fight over this, so if you really want it you'll get it. ;-) > >+ return gFolderDisplay.selectedMessages.every(function(msg) { > >+ return GetIdentityForHeader(msg).archiveEnabled; > > Permission granted to make it one long line or move the { on its own line. ;-) In that case even the "return " can go, no? > Activating this now will mean that all archiving will cease to function, > because archiveEnabled will be false in old profiles? What is the migration > path? Checked current TB trunk with an existing profile: works. From what I can tell, if there is no specific setting, the value of the default pref is used (which is in mailnews.js and set to true): http://hg.mozilla.org/comm-central/rev/0347b42406e6 (nsMsgIdentity.cpp): NS_IMPL_IDPREF_BOOL(ArchiveEnabled, "archive_enabled") -> http://mxr.mozilla.org/comm-central/search?string=NS_IMPL_IDPREF_BOOL -> GetBoolAttribute -> http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgIdentity.cpp#487 BTW: Base bug 542998 is still in flux, will wait for the final checkin which will move the check function to folderDisplay.js (gFolderDisplay). We should definitely stay in sync here; rushing it now would only cost me time later.
Comment on attachment 518104 [details] [diff] [review] patch >+function CanArchiveMsg() I would have expected this to have been called MsgCanArchive() ;-) >+ if (gFolderDisplay.selectedCount == 0) >+ return false; Hmm, this isn't exactly equal to the previous test (selectedMessages.length) >+ return gFolderDisplay.selectedMessages.every(function(msg) { >+ return GetIdentityForHeader(msg).archiveEnabled; >+ }); Nit: incorrect indentation; only 2 spaces for code in an inline function.
Attached patch patch v2 [Checkin: comment 7] (deleted) — Splinter Review
New version now that attachment 518416 [details] [diff] [review] has a positive review
Attachment #518104 - Attachment is obsolete: true
Attachment #518104 - Flags: superreview?(neil)
Attachment #518104 - Flags: review?(mnyromyr)
Attachment #518928 - Flags: superreview?(neil)
Attachment #518928 - Flags: review?(mnyromyr)
Attachment #518928 - Flags: superreview?(neil) → superreview+
Attachment #518928 - Flags: review?(mnyromyr) → review+
Attachment #518928 - Attachment description: patch v2 → patch v2 [Checkin: comment 7]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
No longer blocks: TB2SM
Blocks: 651629
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: