Closed Bug 542998 Opened 14 years ago Closed 13 years ago

Need option to disable Archiving completely

Categories

(Thunderbird :: Folder and Message Lists, enhancement)

enhancement
Not set
normal

Tracking

(blocking-thunderbird5.0 -)

RESOLVED FIXED
Thunderbird 3.3a3
Tracking Status
blocking-thunderbird5.0 --- -

People

(Reporter: tanstaafl, Assigned: squib)

References

(Blocks 2 open bugs, )

Details

(Whiteboard: [gs][datalossy])

Attachments

(4 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: 3.0.1 final

There doesn't appear to be any way to completely disable archiving. I find this quite odd.

Other mail clients that allow archiving have it as an *optional* feature that is *disabled* by default.

I really, really dislike having things forced on me. I do *not* want to archive my messages, and there should be an option to disable it.

Come on guys... please don't shove things down our (I'm a loyal TB user since about version 0.8) throats like this.

Options are good.

Reproducible: Always

Steps to Reproduce:
1. Install Thunderbird
2. Launch Thunderbird
3. Archiving is enabled/active with no way to disable it.


Expected Results:  
Archiving should be *optional* - and in my opinion, *disabled* by default.

I'm filing this as a bug intentionally, because as I see it, it *is* a bug. This should *never* have been enabled by default, much less with no way to disable it.

Also, accidentally hitting the 'A' key with any message(s) selected moves them to an 'Archive' folder *on the same server* - that is silly - the purpose of archiving is generally to help reduce quota usage, no? But that is anothe 'bug' for another day.
Severity: major → normal
Whiteboard: dupme
Well, being able to switch off the archiving feature entirely is suggestion (b) in bug 511741 comment #1, which in the next comment was suggested to be moved into a separate bug. Thus, unless it's filed anywhere else already, that would be *this* bug and therefore not a duplicate.

Also see my bug 511741 comment #3, related to bug 517514, for combining this with the granularity option and UI under development there.
According to Roland in the attached URL, bug 511741 is tracking the ability disable archive, so setting this more specific bug to block.


Note that in the technical sense, this bug is in fact an enhancement: TB has never before had the ability to disable archiving completely. While it does seem reasonable that this feature should have been added when archiving was, this did not happen, so this is a missing feature, not a bug.
Blocks: 511741
Severity: normal → enhancement
Version: unspecified → 3.0
I'll go ahead and confirm that, an unconfirmed bug shouldn't block a confirmed one after all. ;-) The reasoning in comment #2 makes sense, the bug here would have to provide some backend solution for disabling the archive feature first before it can be considered for a first-use warning as suggested in bug 511741.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: dupme
(In reply to comment #2)
> Note that in the technical sense, this bug is in fact an enhancement: TB has
> never before had the ability to disable archiving completely.

Lol... well, it never had *archiving* either.

Regardless, I hope this gets added soon, this really really 'bugs' me...
could someone clarify whether this bug is just about disabling *automatic archiving*, or, also including manual archiving with the "a" key / Archive button?

because the bug's description reads quite differently depending which meaning you take.

i would agree that having automatic archiving newly enabled in TB3 by default, with no warning and no way to turn it off, is a Really Bad Idea. (particularly if it operates on folders other than the Inbox, but that i haven't figured out yet.) either way, the somewhat urgent tone of the original description makes sense. while manual archiving seems to me like a different case.

i found on this page:
http://support.mozillamessaging.com/en-US/kb/Archived+messages
the following statement:
"The frequency of the automatic archiving is based on the mail.server.default.archive_granularity setting, which is set to "1" (monthly) by default. To disable automatic archiving, change the value to "0"."
...however on any other page i've found referencing that setting, it's described only as affecting whether your Archives folder has year/month subfolders or not.

so, is that statement true, and if so, does it solve this bug?
I'm not aware of any "automatic" archiving, it's always a manual process unless a filter option is available already (I only see Move or Copy as filter action but not Archive).

That support article appears to be incorrect. Currently, it is not possible to disable archiving (otherwise this bug would have been closed already), the only effect this preference has is the subfolder structure. A value of "0" simply implies that no subfolders are created in the specified Archives folder.
Depends on: 547493
Have a look at http://mxr.mozilla.org/comm-central/ident?i=archiveGranularity, the values of that preference are defined in nsIMsgIncomingServer.idl:

> 535   /**
> 536    * @{
> 537    * This attribute and constants control the granularity of sub-folders of the
> 538    * Archives folder - either messages go in the single archive folder, or a
> 539    * yearly archive folder, or in a monthly archive folder with a yearly
> 540    * parent folder. If the server doesn't support folders that both contain
> 541    * messages and have sub-folders, we will ignore this setting.
> 542    */
> 543   const long singleArchiveFolder = 0;
> 544   const long perYearArchiveFolders = 1;
> 545   const long perMonthArchiveFolders = 2;
> 546   attribute long archiveGranularity;

The respective handling of archive requests is done in mailWindowOverlay.js in the archiveSelectedMessages function. This would be a good starting point for switching off archiving, e.g., by introducing another (-1) enumerated value to prevent that function from adding any archive actions to the batch.
(In reply to comment #5)
> could someone clarify whether this bug is just about disabling *automatic
> archiving*, or, also including manual archiving with the "a" key / Archive
> button?

I assumed this was about disabling manual archiving.

> because the bug's description reads quite differently depending which meaning
> you take.

It does indeed.

> i would agree that having automatic archiving newly enabled in TB3 by default,
> with no warning and no way to turn it off, is a Really Bad Idea.

I would too. I would in fact capitalize that further.

> i found on this page:
> http://support.mozillamessaging.com/en-US/kb/Archived+messages
> the following statement:
> "The frequency of the automatic archiving is based on the
> mail.server.default.archive_granularity setting, which is set to "1" (monthly)
> by default. To disable automatic archiving, change the value to "0"."
> ...however on any other page i've found referencing that setting, it's
> described only as affecting whether your Archives folder has year/month
> subfolders or not.

I was considerably surprised (not to say alarmed) to read that statement just now in the article linked, which appears to say that TB does in fact turn on automatic archiving by month, especially because this has not taken effect on any of my (two dozen, mostly IMAP) accounts (some added before upgrade to 3.0, some after), even though the pref given is still at its default value of 1.
I filed a clarification doc bug (bug 547493) on that, blocking this one.
The granularity is "per month" by default, you still have to invoke "Archive" manually (I'd have noticed if it's archiving automatically once every month).
Ok... well, that article is actually the help file about archiving, it's what you get when you choose "Thunderbird Help" from the Help menu, and look under "General - Archived messages". And it does appear to be completely wrong.

That might explain Charles Marcus' original description of this bug a little better?

Because otherwise I don't really see anything being "shoved down our throats" etc. - just don't press the Archive button. There are even extensions already to remove the button and remap the "a" key.

Charles?
Jeff, you may want to read the motivation in bug 511741 and the support thread Nathan has linked to in the heading of this bug. The issue is an /accidental/ archiving. Thus, if someone doesn't need the feature, it makes sense to provide an option to disable it.
Whiteboard: [gs]
Blocks: 547493
No longer depends on: 547493
personally i won't use the archiving because its date-based folder structure doesn't suit me. so it's fine with me if there's a way to disable it altogether. a better solution might be to improve it so it's more useful.

the issue of accidental archiving is only caused by the single-key shortcut. therefore i would rather see a solution addressing single-key shortcuts in general, rather than disabling the individual features they invoke.

but i don't feel very strongly about either of these, either way.

i only commented after reading in the help menu that the archiving would happen automatically. i guess it turns out it doesn't. as far as i'm concerned it's not really that urgent to disable manual archiving, i can learn to ignore it. now that nathan has filed a bug on the documentation, i guess i'm done here.

i'm not sure that the original reporter's issue was with accidental archiving, maybe he had the same misunderstanding as i did. so let's hear from him.
(In reply to comment #10)
> Ok... well, that article is actually the help file about archiving, it's what
> you get when you choose "Thunderbird Help" from the Help menu, and look under
> "General - Archived messages". And it does appear to be completely wrong.
> 
> That might explain Charles Marcus' original description of this bug a little
> better?
> 
> Because otherwise I don't really see anything being "shoved down our throats"
> etc. - just don't press the Archive button. There are even extensions already
> to remove the button and remap the "a" key.
> 
> Charles?

Ok, I was in a really bad mood when I made that comment, so I apologize for that. But yes, I want a way to completely disable archiving. I don't want our users to 'accidentally' archive anything, because no matter what the key/key combination is, some of them will do it.

And fyi, if Archiving *had* been made fully automatic as described in that help article - well, we probably wouldn't be having this conversation because I probably would have never used TB again - and this is from someone who loves TB, and has been using it in our 50+ small company exclusively since about version 0.8.

When features like this are designed, consideration really needs to be given to those of us who are trying to use TB in a corporate environment.

Hopefully, eventually, TB (and FF) will become enterprise ready by supporting Group Policies (wow - I just checked and there doesn't even seem to be a bug open for this???), and when that happens, Admins will *require* the ability to define things like this.

For example, I still maintain that one of the more useful purposes for archiving is quota management (move messages off a server to relive an over quota issue), so at a minimum, the archiving feature needs to be a lot more robust (able to define a completely different location for archiving messages, like, for example - the Local Folders, an archive server (slower/less expensive storage), etc...

[OT]
There are/were a few things about TB3 that really annoyed me when I first started using it, and I felt like the devs were ruining a potentially awesome update of TB by forcing some UI changes on everyone - Tabs is one example. I really hated them at first - I still don't like them, but have figured out how to disable them for everything but the Calendar, so I can live with that for now, even though I still want the option to open the Calendar in the current Tab to achieve old TB2 behavior. Another example is totally ignoring my carefully selected offline folder settings for my 16+ IMAP accounts and just forcing everything to full offline mode. Honestly, that one thing is probably responsible for most every 'rant' I've made since TB3 went gold... I still get infuriated when I recall figuring out what happened and that it was intentional.
[/OT]
ok, fair enough. so this bug is about completely disabling manual archiving, in case users might do it accidentally.

in the meantime, a work-around for some might be the "keyconfig" addon, which lets you completely disable the Archive key shortcut (and any other "dangerous" keys, like "j"...)
http://forums.mozillazine.org/viewtopic.php?t=72994
http://kb.mozillazine.org/Keyconfig_extension:_Thunderbird

also, these two addons let you remove the Archive button from the header toolbar:
https://addons.mozilla.org/en-US/thunderbird/addon/45601
https://addons.mozilla.org/en-US/thunderbird/addon/13564

good luck...
Thanks for the pointers to the extensions... I already found/installed one  that lets me remap the key from 'A' to SHIFT-a, to minimize the chances of this happening, but doing things like this manually is a *lot* of work in a corporate environment...
(In reply to comment #5)
> could someone clarify whether this bug is just about disabling *automatic
> archiving*, or, also including manual archiving with the "a" key / Archive
> button?
> 
> because the bug's description reads quite differently depending which meaning
> you take.
> 
> i would agree that having automatic archiving newly enabled in TB3 by default,
> with no warning and no way to turn it off, is a Really Bad Idea. (particularly
> if it operates on folders other than the Inbox, but that i haven't figured out
> yet.) either way, the somewhat urgent tone of the original description makes
> sense. while manual archiving seems to me like a different case.
> 
> i found on this page:
> http://support.mozillamessaging.com/en-US/kb/Archived+messages
> the following statement:
> "The frequency of the automatic archiving is based on the
> mail.server.default.archive_granularity setting, which is set to "1" (monthly)
> by default. To disable automatic archiving, change the value to "0"."
> ...however on any other page i've found referencing that setting, it's
> described only as affecting whether your Archives folder has year/month
> subfolders or not.
> 
> so, is that statement true, and if so, does it solve this bug?


the statement is not true, there is NO automatic archiving, Standard8 aka Mark Banner corrected http://support.mozillamessaging.com/en-US/kb/Archived+messages

thanks for the heads up!
Glad the support page is fixed, but we still need a way to disable Archiving completely.
(In reply to comment #17)
> Glad the support page is fixed, but we still need a way to disable Archiving
> completely.

This bug is a bit confusing.  bug 511741 is going to handle disabling the key combination for archiving.  If the header button customization gets finished then users will be able to remove the button giving users few options to archive an email.

I'm a bit unsure what is being requested beyond what is already happening.  AFAIK there aren't other options like this that disable Junk, Reply, Forward, or other functions.  With the ability to disable the key combo and the ability to remove the button; Archiving will be more removable than Junk currently is.
Well, features like this absolutely *must* have a very simple, non-confusing boolean UI option, like:

[X] Disable Archiving for this account

so that those of us using TB in a corporate environment can disable Archiving by simply modifying this one pref in user.js, rather than having to manually massage the GUI for every users profile.

The TB devs really, really need to start thinking in terms of how something like this will impact corporate users. Meaning, always ask yourself "how can we jigger this option so that corporate admins will be able to control this feature using Group Policies?". Making it a simple user.js pref means I can still apply this option manually now, and making it controllable through GPO when they are supported will then be trivial.

That said, I see no reason why this simple boolean option couldn't simply be the UI method to: a) disable the keyboard shortcut *and*; b) completely *hide* the toolbar buttons - message header and main - so that the user wouldn't even be able to add the buttons to a toolbar if they tried. This would also be much more logical than having to have an FAQ somewhere that support people would always be having to refer people to telling the user:

"If you want to disable archiving, you need to:

1. Disable the Archive key shortcut, and

2. Remove the toolbar buttons from both of your toolbars (if they are present) and resist the urge to re-add them later.

and then telling us corporate users "Sorry, you'll just have to put up with this mess for now.

If you're going to fix this, fix it right, the first time.
I'm applying a couple of patches for each release, which can be done in a batch and include removal of the archive shortcut (and button, either by hacking the theme or adding it to the userChrome.css of the default profile). So, it's not convenient but would do as a workaround for the time being.

(In reply to comment #18)
> AFAIK there aren't other options like this that disable Junk, Reply, Forward,

Uhm, neither of those moves the message by default - not even the Junk button, which will just mark the message unless it has been configured otherwise. Thus, you provided (unintentionally) a good parallel, given that moving messages when marked as junk can be turned off as well in the account settings.
(In reply to comment #20)
> I'm applying a couple of patches for each release, which can be done in a
> batch and include removal of the archive shortcut (and button, either by
> hacking the theme or adding it to the userChrome.css of the default profile).
> So, it's not convenient but would do as a workaround for the time being.

Cool - care to share? I'd also be intersted in the the userChrome hack to remove the buttons, since the boss may not agree to me patching the binary at install time?

;)
You don't need to hack the binary, just XUL and CSS code. After installation, put this patch (I had the shortcut disabled already in my collection, but have added hiding the archive buttons for your version) into the chrome folder of the installation and run the following script there:


unzip messenger.jar content/messenger/mailWindowOverlay.xul
unzip classic.jar skin/classic/messenger/messageHeader.css \
                  skin/classic/messenger/multimessageview.css
patch -p0 < charles.patch
zip -0 messenger.jar content/messenger/mailWindowOverlay.xul
zip -0 classic.jar skin/classic/messenger/messageHeader.css \
                   skin/classic/messenger/multimessageview.css


which you've put before into a text file named, let's say, charles.sh - running it from command line (in Linux) or a Cygwin bash shell (Windows, make sure to install the zip and patch packages) should perform the necessary modifications. Obviously, don't have Thunderbird running at that time. Then, repackage the modified installation and deploy to your users. Since the original JAR files have been changed, automated partial updates will fail, thus you'll have to make a full install for each subsequent release and run the script again.
Awesome... thanks!
(In reply to comment #20)
> I'm applying a couple of patches for each release, which can be done in a batch
> and include removal of the archive shortcut (and button, either by hacking the
> theme or adding it to the userChrome.css of the default profile). So, it's not
> convenient but would do as a workaround for the time being.

This is great!  It would be good to have a better way to connect with enterprises who want to share scripts and such.  Perhaps the knowledge base could link to something like this in the developer wiki?

I'm going to mark this as WONTFIX now because there are definitely ways for enterprises, as shown, who don't want this behavior to remove it completely.  bug 511741 is going to handle the first use experience for regular consumers so I don't think bug is necessary.

> (In reply to comment #18)
> > AFAIK there aren't other options like this that disable Junk, Reply, Forward,
> 
> Uhm, neither of those moves the message by default - not even the Junk button,
> which will just mark the message unless it has been configured otherwise. Thus,
> you provided (unintentionally) a good parallel, given that moving messages when
> marked as junk can be turned off as well in the account settings.

Yes, off topic, a couple things are wrong with that.  Mark as junk is an uncommon behavior.  Every major web mail client and desktop client moves junk to the junk folder by default.  We have a support problem with our default; it is incorrect, we just haven't changed it.  The options for Junk do not disable or turn off the Junk feature, it only alters it's behavior from 'mark as junk' to 'move to junk'.
I'm happy to see this hasn't been marked as WONTFIX as yet.  It's nice that there's a kludge workaround but the core issue still remains: if archiving is activated inadvertently, it can cause serious dataloss and there is no undo available.  The fact that the workaround breaks automatic updating is also, I'm sure, a problem for people.

As for "Mark as Junk" being an "uncommon" behaviour, well... not everyone wants to archive their email, especially not in the way it's set up to be done by default now in Thunderbird.  So archiving can definitely be considered uncommon.
Of course undo is available.
(In reply to comment #25)
Undo does work and with bug 511741 we'll be offering an "undo plus turn off the keyboard shortcut" aka disable dialog on first use of archiving.

> As for "Mark as Junk" being an "uncommon" behaviour, well... not everyone wants
> to archive their email, especially not in the way it's set up to be done by
> default now in Thunderbird.  So archiving can definitely be considered
> uncommon.

Right, the archive feature was designed with the idea that we would be able to extend the way archives are handled.  For instance bug 533815 will keep the folder structure and there are other ideas that we want to make it easier to implement.  The current archive is a one-size fits all but was made to be modified so it works for more people.  I don't think we'd want to disable it because it doesn't archive exactly as we want yet.  I'd really like to see it archive by people, groups of people, and mailing lists.  That's probably all after the 3.1 release.
I emphatically agree with Joey... Bryan, *please* don't mark this as WONTFIX, this absolutely *needs* to be fixed.

Bug 511741 actually refers back to *this* bug to 'handle' the b) option to 'disable archiving completely', so referring to it as an excuse to WONTFIX this bug is unreasonable.

Bryan - some enterprises (maybe even a lot) will *absolutely* *insist* on a way to completely disable archiving, including eliminating the toolbar buttons and the Archive folder (if it were to inadvertently get created somehow).

This is a good example of what I mean when I keep saying, please, *please* start considering how *enterprise* admins think about these things. Having to manually apply a script/patch is absolutely, positively *not* a long term solution to this issue.

Having a simple, single pref that is easily toggled via Group Policies is what any admin will want and expect (once support for GPOs is achieved).
I never use or intend to use Archiving. I'd like to recover the screen real estate in the Folders Pane *and* eliminate issues with inadvertent touching of the related Archiving hot keys. I think complete disabling of Archiving is important and it should be an easy fix.
> Of course undo is available.

Please tell me how to do it then - I want the message back in the folder it came from with the same 'order received'.
I just spent a panicked hour and half recovering from an archive operation of 20k email that happened in concert with a **** load of indexing and several crashes. I love TB and appreciate all the hard work that has gone into making this great piece of software.  Pls, don't get enamored with your own ideas.  You almost certainly have not thought out all the ways ppl are using it.  Anything that has so much power for damage should be introduced with great caution and should never be released without a way to restore a previous state.  thanks.
So, 3.1 is out and still no option to disable archiving.

Thanks for listening TB devs...
Considering enterprise deployments is very important, but fixing this particular design bug (yep, that's what it is. sorry) doesn't rely upon that important consideration. It just adds to the urgency. The entire user base, both those inside and outside of administered corporate environments, suffer the same confusion and risk to their established message storage folder structures caused by this implementation. 

It makes no sense that an accidental striking of the "a" key hauls whichever messages happen to be currently selected off to some new top level hierarchy with no user notification. Even with Undo available, if it happens by accident it's not unlikely that the user won't know to use it.

Regarding allowing opt-out at installation or update time, this only helps those users who know to watch out for this. For new users importing existing storage structures into their shiny new Thunderbird, if they have to opt-out of this then it's just a trap.

It is very, very nice that some participants have provided or pointed to solutions users may attempt to fix this themselves, but these solutions do not address the basic design flaw. 

Thanks very, very much for your work and for listening, developers. For valuable exposure to some end user pleadings on this issue, I encourage you to read the comments on one of the cases on this issue opened on getsatisfaction.com by following this link: http://gsfn.us/t/nswy
And thank YOU, Charles, for keeping on this.
Here's another discussion in which end users are searching for solutions to this design bug. It's very interesting (I say, even though I'm a participant):
http://gsfn.us/t/o8lf
(In reply to comment #34)
> It makes no sense that an accidental striking of the "a" key hauls whichever
> messages happen to be currently selected off to some new top level hierarchy
> with no user notification. Even with Undo available, if it happens by accident
> it's not unlikely that the user won't know to use it.

Is this something bug 511741 (notify on first use of feature; disable keyboard shortcut) will be able to fix? If so, it's not technically in scope for this bug, which is about implementing an option to completely disable not only the keyboard shortcut for this feature, but the entire feature itself.
Well, there's some overlap. One concerns the other. The reason for this bug is because of the behaviors described and addressed both here and in bug 511741. 

For example, if on first use notification the user has enough presence of mind to turn off the feature, from their perspective it'd gone, removed. Annoyance addressed. However, first use notifications are problematic, since the user may NOT realize the implications of the decision being presented, or even what it is. Remember, if this thing pops up on an accidental key "a" strike, they were probably in the midst of doing something else with the product; some normal user task. All the sudden we jerk them out of that and ask them to make a one-time-only configuration decision with no going back. Annoyance caused.
I just re-read all of the comments in  this bug, and I'm really puzzled. I don't know how many more times or different ways this needs to be said - and I especially don't see how anyone can comment that the bug Summary/Title is in any way incomplete or unclear.

PLEASE PROVIDE A SIMPLE WAY TO COMPLETELY AND UTTERLY DISABLE THE NEW ARCHIVING FEATURE.

The hacks/workarounds described in this bug commentary are just that - hacks and workarounds to try to minimize and hide the nasty effects of this bug - and no, bug 511741 is not even close to a satisfactory solution to this bug.

The fact that there is an 'undo' feature is fine - *if* the user knows about it, understands it, and actually uses it *immediately* - otherwise, it is totally useless.

I have had to spend multiple HOURS of my time since 3.0 was released, explaining this new 'feature' to many DOZENS of panicked people I support, and how to find their mysteriously LOST messages.
I agree 100% with Cahrles and others.

1) Don't futz around with a patch for control-A, just make Archiving completely optional; and:

2) Make it _real_ archiving: external folder tree, with messages saved as individual *.eml files, using Subject (and numbering) as file name.

As implemented, Archiving is just another mess of mboxes. Not the best method for secure archives IMO.
I agree 100% with Charles and others.

1) Don't futz around with a patch for control-A, just make Archiving completely optional. ADD IT AS AN OPTION TO THE MESSAGE MENU.

Sorry for shouting, but it seems to me a no-brainer: _any_ new feature should either be offered as a toggle on/off option (per account or globally), or added as a menu option.

That's just good design.

And:

2) How about REAL archiving? You know, external folder tree, with messages saved as individual *.eml files, using Subject (and numbering) as file name.

As implemented, Archiving is just another mess of mboxes. Not the best method for secure archives IMO.
I think we should want this for 3.3. It is datalossy for some people - though probably mostly affects those who never (intentionally) use it/never need it/etc
blocking-thunderbird5.0: --- → ?
Whiteboard: [gs] → [gs][datalossy]
Attached patch Real patch to fix this (obsolete) — Splinter Review
This patch adds a hidden per-identity pref ("archive_enabled") that defaults to true and disables everything when it's false. No tests yet, but that should be straightforward.

This also fixes an issue where the multi-message/thread summaries showed the archive button even in the archive folders.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
protz: I imagine you may want to hook into this for Thunderbird Conversations. The relevant function to check for this is CanArchiveMsg(). You can see the code for it in attachment 515439 [details] [diff] [review]. If that implementation would cause you problems, let me know and I can adjust it.
Many thanks for hooking me up on this. I won't be able to reuse the CanArchiveMsg() function as is, but the logic is very simple, so I'll just make a version for my own personal use (I mostly need to past a list of msgHdrs rather than using the selected ones).

Looks fine for me, as soon as this lands, I'll update Thunderbird Conversations to take this into account.
Thanks very much for implementing this Jim!

One question - does this allow you to delete the special 'Archive' folder once it is disabled?
No, the patch only disables the "archive" functionality if the preference
is modified. That's certainly desired given that the user better deletes the folder manually if he or she wants to do so.

>--- a/mailnews/base/public/nsIMsgIdentity.idl
>+++ b/mailnews/base/public/nsIMsgIdentity.idl

You need to assign a new uuid at the top of an IDL file when changing an interface.

>--- a/mailnews/base/util/nsMsgIdentity.cpp
>+++ b/mailnews/base/util/nsMsgIdentity.cpp

I'm wondering why there is no COPY_IDENTITY_BOOL_VALUE for ArchiveEnabled in nsMsgIdentity::Copy(), but then there isn't one for ArchiveGranularity either, so that may only be required for options which also have a UI elements. Thus, that addition would have to be done in bug 607295 if my assumption holds.
(In reply to comment #45)
> Many thanks for hooking me up on this. I won't be able to reuse the
> CanArchiveMsg() function as is, but the logic is very simple, so I'll just make
> a version for my own personal use (I mostly need to past a list of msgHdrs
> rather than using the selected ones).

Maybe this isn't the bug to do it in, but perhaps all these functions (CanArchiveMsg, CanMarkMsgAsRead, SelectedMessagesAreRead, etc.) should optionally take an array of msgHdrs. Even though most of these functions are very simple, I imagine they start to add up when a person needs to reimplement them all.

If that sounds useful (other people can weigh in of course), I could work on that in a separate bug.
(In reply to comment #47)
> No, the patch only disables the "archive" functionality if the preference
> is modified. That's certainly desired given that the user better deletes the
> folder manually if he or she wants to do so.

Correct. I'm more than a little hesitant to go around deleting people's folders, especially since I have no way of knowing which folders the user doesn't want (for instance, a user could have created an Archives folder on their own, and then they got annoyed because Thunderbird added a bunch of subfolders to it, messing up their organization).

I plan on doing bug 511741 after I finish this and bug 607295, so hopefully from 3.3 on, people won't run into a situation where they have accidental archive folders.

> >--- a/mailnews/base/public/nsIMsgIdentity.idl
> >+++ b/mailnews/base/public/nsIMsgIdentity.idl
> 
> You need to assign a new uuid at the top of an IDL file when changing an
> interface.

Ah, right. Patch forthcoming.

> >--- a/mailnews/base/util/nsMsgIdentity.cpp
> >+++ b/mailnews/base/util/nsMsgIdentity.cpp
> 
> I'm wondering why there is no COPY_IDENTITY_BOOL_VALUE for ArchiveEnabled in
> nsMsgIdentity::Copy(), but then there isn't one for ArchiveGranularity either,
> so that may only be required for options which also have a UI elements. Thus,
> that addition would have to be done in bug 607295 if my assumption holds.

Yeah, I'll take a look at that when I get to bug (which should be the near future).
Oh, I see. The archive folder can't be deleted manually either. I think that might really be a different bug for the following reasons:

1) You should be able to delete *any* folder (or at least attempt to; your IMAP 
   server may not let you).
2) You can't delete the "Junk" folder, but you can delete Gmail's "Spam" folder,
   which is bizarre, and makes me think that there's just a list of names for
   sacred folders. I wonder if this works in other languages or not...

Special folders should probably have something a little more emphatic than "Are you sure?" though. Something like "This is a special folder, and deleting it may break important things! Do you want to delete it anyway?"
If you disable the archive functionality, I think you should clear the archive pref as well, which should clear the archive flag from the archive folder, and allow the user to delete it (except for the gmail all mail/archive case, where the server tells us the All Mail folder is the archive folder, but I don't think you want to delete the all mail folder anyway).
Which pref is that? The location for the archive folder? I didn't think we should delete the pref so that people could restore the old state more easily. Maybe that's not important.

Wouldn't clearing the pref be handled in another bug though (e.g. bug 607295)? This patch just adds a hidden pref, so there's no UI for it yet.
Second question...

Is this planned to be a hidden pref forever? Or is this just the first step?

The reason I ask is, imo, this should *not* be a hidden pref, it should be right there in the UI.

I get questions *all the time* from people who I have turned on to Thunderbird about how to disable it.
Charles, bug 607295 attachment 515572 [details] should answer your question.
As a golden rule of thumb, *read* first, *then* complain... ;-)
Dang... sorry, and many thanks to Jim for working on this!
(In reply to comment #52)
> Which pref is that? The location for the archive folder? I didn't think we
> should delete the pref so that people could restore the old state more easily.
> Maybe that's not important.

Yes, the location for the archive folder, i.e., the folder URI. 
> 
> Wouldn't clearing the pref be handled in another bug though (e.g. bug 607295)?
> This patch just adds a hidden pref, so there's no UI for it yet.

Whichever bug you want to do it in is fine with me...but I think people who turn off archiving are going to want to delete the archive folder. Perhaps you could clear the archive folder flag on the archive folder and disable the code that sets the flag on the folder based on the URI, but it seemed like changing the folder pref would be easier.
(In reply to comment #56)
> Whichever bug you want to do it in is fine with me...but I think people who
> turn off archiving are going to want to delete the archive folder. Perhaps you
> could clear the archive folder flag on the archive folder and disable the code
> that sets the flag on the folder based on the URI, but it seemed like changing
> the folder pref would be easier.

I may do this the latter way, since it shouldn't be too hard to check the archive_enabled pref before we check the archive folder location pref.

Anyway, once I get this and bug 607295, it'll be time for bug 511741, since I'll be able to hook the first-use dialog into the new UI so people can mess with their archive hierarchy (or turn it off) before anything actually gets archived.
Attached patch Cleanup and add tests (obsolete) — Splinter Review
Ok, this fixes the UUID issue mentioned above and adds some tests (plus a couple other tiny things like adding comments). I think this is about ready for review.

:asuth, do you mind taking a look at this? Worth considering are comment 51 and comment 56, though my current plan is to address that in bug 607295. If you think that's a bad move, let me know.
Attachment #515439 - Attachment is obsolete: true
Attachment #515850 - Flags: review?(bugmail)
Oh also worth considering is the COPY_IDENTITY_BOOL_VALUE issue in comment 47. Right now I'm just following the principle of "when in Rome", but maybe we should really be copying those prefs even though they're hidden.
I thought about it some more and I realized that this bug should handle setting/clearing the archive folder flag. I'm really not sure if this is the right way to do it, but it seems to work. It requires a restart to clear the state, though it seems like this is the case for all the other special folders, so I'm not too worried about that...

Also, this patch properly hides the "Archive" context menu item when archiving is disabled.
Attachment #515850 - Attachment is obsolete: true
Attachment #516834 - Flags: review?(bugmail)
Attachment #515850 - Flags: review?(bugmail)
Blocks: 607295
Whoops, I forgot to get rid of some debug code...
Attachment #516834 - Attachment is obsolete: true
Attachment #516837 - Flags: review?(bugmail)
Attachment #516834 - Flags: review?(bugmail)
Comment on attachment 516837 [details] [diff] [review]
Remove debug printf
[Checked in: Comment 79]

I don't see an explicit clarkbw sign-off so requesting one.  Likewise, I don't see an explicit bienvenu sign-off on the preference living on the identity and that's above my paygrade, so we need one of those too.

fancy review:
http://reviews.visophyte.org/r/516837/

exported review:
on file: mail/base/content/mail3PaneWindowCommands.js line 376
>       case "cmd_markAsFlagged":

cmd_markAsFlagged should probably not be falling through to the archive logic
here...

This bug previously existed, but it shouldn't be too hard to just move that
case to where button_mark and cmd_tag live.  Thanks!


on file: mail/base/content/mailWindowOverlay.js line 1531
> /**
>  * Checks if the selected messages can be archived. This depends on what folder
>  * they're in and whether archiving is enabled for that identity.
>  *
>  * @return true if all selected messages can be archived, false otherwise
>  */
> function CanArchiveMsg()
> {
>   if (gFolderDisplay.selectedCount == 0)
>     return false;
>   if (!gFolderDisplay.displayedFolder ||
>       gFolderDisplay.displayedFolder.isSpecialFolder(
>         Components.interfaces.nsMsgFolderFlags.Archive, true))
>     return false;
>   return gFolderDisplay.selectedMessages.every(function(msg) {
>       return getIdentityForHeader(msg).archiveEnabled;
>     });
> }

Please move this onto FolderDisplayWidget next to the very similar
canDeleteSelectedMessages.  There are already enough methods in the global
soup :)


on file: mailnews/base/src/nsMsgAccountManager.cpp line 1544
>               printf("Clearing archive flag\n");

(leftover debugging code)
Attachment #516837 - Flags: ui-review?(clarkbw)
Attachment #516837 - Flags: superreview?(bienvenu)
Attachment #516837 - Flags: review?(bugmail)
Attachment #516837 - Flags: review+
(In reply to comment #62)
> Comment on attachment 516837 [details] [diff] [review]
> Remove debug printf
> 
> I don't see an explicit clarkbw sign-off so requesting one.  Likewise, I don't
> see an explicit bienvenu sign-off on the preference living on the identity and
> that's above my paygrade, so we need one of those too.

Yeah, some of this stuff is a little worrisome to me since identities are confusing and I think there are some already-existing bugs in there (e.g. it seems like creating a new Gmail account on trunk doesn't set the archive_granularity properly, which I mentioned on m.d.a.t).

> 
> fancy review:
> http://reviews.visophyte.org/r/516837/
> 
> exported review:
> on file: mail/base/content/mail3PaneWindowCommands.js line 376
> >       case "cmd_markAsFlagged":
> 
> cmd_markAsFlagged should probably not be falling through to the archive logic
> here...
> 
> This bug previously existed, but it shouldn't be too hard to just move that
> case to where button_mark and cmd_tag live.  Thanks!

While we're here, what are button_file and cmd_file? It seems like they're totally unused.

I've got the other stuff fixed and I'll upload a new patch once I know what to do with (button|cmd)_file (probably just delete them).
Blocks: 499155
Hmm, based on bug 499155, it seems that just disabling the archive button in the archive folder is bad, since it makes it impossible to re-archive. I suppose we could update the canArchiveSelectedMessages property to take that into account. Ideas?
Comment on attachment 516837 [details] [diff] [review]
Remove debug printf
[Checked in: Comment 79]

I'm fine with the hidden pref for now.  I'm sure we could make up a quick extension that changes the pref on install and sets it back on uninstall just in case we don't get the UI settled.

As Jim said, any UI for modifying the pref seems like it would be handled in bug 511741 and not in this bug.  I have a quick mockup I'll attach to this bug just to show a possible UI but I need bienvenu to let me know where this is living (identities or accounts) such that I can know what other possible pref UIs would be required.

I'm assuming I can place the pref option where the mockup suggests, in accounts, and then I might have to also have it in identities as well.  Perhaps the accounts one will just affect the identities as a whole; might need an extra dialog to handle that question.
Attachment #516837 - Flags: ui-review?(clarkbw) → ui-review+
here is said mockup which shows the checkbox in the account settings window.  If this is really stored per identity we'll probably need this copied to those settings dialogs as well.  I'd like to have a checkbox be in the account settings main dialog since the identities window is likely a cavern most people don't enter.
(In reply to comment #62)

> I don't see an explicit clarkbw sign-off so requesting one.  Likewise, I don't
> see an explicit bienvenu sign-off on the preference living on the identity and
> that's above my paygrade, so we need one of those too.
If we want a per-account setting for disabling archiving, it should live on the identity to be consistent with the granularity setting. I can't imagine anyone wanting to turn off archiving only partially, but that's really a question for Bryan (i.e., I suspect people either want to be able to archive with 'a' all the time, and see the archive button on the toolbar, or never see it). Which means, there should be UI to control the default value for "mail.identity.default.archive_enabled".

I believe the account settings UI for special folders is only poking at the default identity for the account.
(In reply to comment #67)
> If we want a per-account setting for disabling archiving, it should live on the
> identity to be consistent with the granularity setting.

It does, nsIMsgIdentity.idl is respectively extended here. The UI draft shown
in bug 607295 adds a checkbox in Copies & Folders, which should be available in secondary identities as well (assuming the same overlay is used).

As for mail.{identity,account,server}.default.* settings being accessible, that's probably more of a general problem that could equally apply to other account-manager prefs.
(In reply to comment #68)
> 
> It does, nsIMsgIdentity.idl is respectively extended here. The UI draft shown
> in bug 607295 adds a checkbox in Copies & Folders, which should be available in
> secondary identities as well (assuming the same overlay is used).
Yes, I wasn't disagreeing - the patch is doing the right thing in that respect.
> 
> As for mail.{identity,account,server}.default.* settings being accessible,
> that's probably more of a general problem that could equally apply to other
> account-manager prefs.
The subject of this bug implies that the user should be able to turn it off completely, and not have to poke every account created...
My plan is to allow global disabling of the archive command on first use (bug 511741). Is that sufficient? You'd have to go through all your identities individually if you wanted to change that later, but I doubt most people are switching back and forth very often.
(In reply to comment #70)
> My plan is to allow global disabling of the archive command on first use (bug
> 511741). Is that sufficient? You'd have to go through all your identities
> individually if you wanted to change that later, but I doubt most people are
> switching back and forth very often.

that's probably the best we can do. People tend to ignore one-time UI (Bryan can correct me if I'm wrong) but it's hard to do anything that makes everyone happy.
Longer-term, we've had some discussions on tb-planning about the "omni-prefs" window which would resolve this by having a global checkbox and a dropdown that would let you select per-account/identity settings. See the "writing" section here for one possible UI: http://clarkbw.net/designs/mega-prefs/
Ahem, user advocate here, bumping in. 

I hope that these recent comments do not mean that existing installs that
already have the archive folders and "a" single hot-key trigger won't be
addressed. Many, if not most of us are long past first use, and would just like
to get rid of this land mine. 

Thanks very much for whatever help you  render. And please excuse the
interruption.
Jim's patch here allows to enable or disable archiving for a specific account, which would equally apply for any account that already exists. Bug 511741 may achieve a solution where archiving is globally disabled when archiving is run
for the first time, that's the only component for new/upgraded installations.
Thanks rsx11m. As long as we can still turn off this feature, provisions for allowing it (for whatever use case could possibly find it useful) are absolutely no problem!
Attachment #516837 - Flags: superreview?(bienvenu) → superreview+
So, for those of us with 20+ accounts, we have to disable it 20+ times? Then delete 20+ Archive folders? Not ideal, but definitely better than what we have available now, so I'll take what I can get... ;)

Also - I'm assuming there will be a way to disable it for new accounts too?
Both can be done setting mail.identity.default.archive_enabled to false, every account/identity not having a custom setting would inherit that, existing or new (that was the point made in comment #67).
For what it's worth, I'll probably be one of the folks who only has archiving enabled for some accounts. I'm sure I'll leave it enabled on my Gmail account, but I don't particularly want it on my non-Gmail account.

Anyway, since this has all the requisite reviews, adding checkin-needed.
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/0347b42406e6
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
blocking-thunderbird5.0: ? → -
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 3.3a3
Attached patch Fix review comments (obsolete) — Splinter Review
Gah, I totally forgot to upload this patch before marking it as checkin-needed. These changes address asuth's review in comment 62.

I'm going to file followup bugs for my questions at the end of comment 63 and in comment 64.
Attachment #518415 - Flags: review?(bugzilla)
Actually... this is a bit cleaner than the above patch. Same story, though.
Attachment #518415 - Attachment is obsolete: true
Attachment #518416 - Flags: review?(bugzilla)
Attachment #518415 - Flags: review?(bugzilla)
Attachment #518416 - Flags: review?(bugzilla) → review+
Marking checkin-needed for the updated patch (attachment 518416 [details] [diff] [review])
Keywords: checkin-needed
(In reply to comment #82)
> Marking checkin-needed for the updated patch (attachment 518416 [details] [diff] [review])

Checked in: http://hg.mozilla.org/comm-central/rev/5294592f5499
Keywords: checkin-needed
Attachment #518416 - Attachment description: Fix comments (once more but this time with feeling) → Fix comments (once more but this time with feeling) [Checked in: Comment 83]
Attachment #516837 - Attachment description: Remove debug printf → Remove debug printf [Checked in: Comment 79]
Attachment #432337 - Attachment description: Post-install patch for shortcut & button → Post-install patch for shortcut & button [TB 3.1.x]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: