The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
MailNews: General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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).
Blocks: 360488
(Assignee)

Comment 1

6 years ago
messageWindow.js: Forget it, we don't have an Archive button.
nsContextMenu.js: Our file is called mailContextMenus.js.
(Assignee)

Comment 2

6 years ago
Created attachment 518104 [details] [diff] [review]
patch

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 3

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

Comment 4

6 years ago
(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 5

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

Comment 6

6 years ago
Created attachment 518928 [details] [diff] [review]
patch v2 [Checkin: comment 7]

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)

Updated

6 years ago
Attachment #518928 - Flags: superreview?(neil) → superreview+

Updated

6 years ago
Attachment #518928 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 7

6 years ago
Comment on attachment 518928 [details] [diff] [review]
patch v2 [Checkin: comment 7]

http://hg.mozilla.org/comm-central/rev/59cc6cab8244
Attachment #518928 - Attachment description: patch v2 → patch v2 [Checkin: comment 7]
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
No longer blocks: 360488

Updated

6 years ago
status-seamonkey2.1: ? → ---
(Assignee)

Updated

6 years ago
Blocks: 651629
You need to log in before you can comment on or make changes to this bug.