79561 - Offline: 2 prefs aren't saved: "When I create new folders select them for offline use" and "Remove messages older than"

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Backend, defect, P2)

Description

[grylchan]

Using 2001050804 build on WinNT 4.0 and
2001050808 on mac

Clicking on the checkboxes for the Offline&Disk Space prefs : 
"When I create new folders select them for offline use" and 
"Remove messages older than X days" results in neither of them being
saved once you exit the pref settings.

Steps to Reproduce:
1) Bring up Mail&News account settings. 
2) Select one of your mail accounts
3) Click on the 'Offline&Disk Space' pref 
4) Click the check box(s) for either one of these prefs
   "When I create new folders select them for offline use"
   "Remove messages older than X days"
5)Click Ok
6)Go back to the Mail&News account settings
7)Select the same mail account and click on 'Offline&Disk Space' pref 

Actual Results: The two checkboxes aren't checked
Expected Results" The two checkboxes to have x's in them

possibly related to bug 79554  ?

Comment 1

[Diane Sun]

different

Comment 2

[scottputterman]

nominating

Comment 3

[Diane Sun]

"When I create new folders select them for offline use"  is identity based 
preference.  I'll need a preference/attribute from the  backend.

There are two "Remove messages older than X days", one for imap, the other for 
nntp.  Since we change to not showing them at the same time,  I can use the same   
preference.  But I need to make sure that backend can identify which sever type 
it goes to. 

Comment 4

[David :Bienvenu]

identity-based? I thought "create new folders select them for offline use"
 would be server based, and in fact, would only be for imap servers (you can't
create new newsgroups, for example, so that pref wouldn't make any sense for
newsgroups). I'm confused.  I'll probably just add it as an attribute of imap
servers soon, so if that's wrong, please let me know.

"Remove messages older than X days" is only implemented for news servers, and is
part of the retention settings attribute of nsIMsgIncomingServer.

look at nsImsgIncomingServer.idl. See that attribute retentionSettings that's an
nsIMsgRetentionSettings

then, look at the definition of retention settings:

interface nsIMsgRetentionSettings : nsISupports
{
	const unsigned long nsMsgRetainAll = 1;
	const unsigned long nsMsgRetainByAge = 2;
	const unsigned long nsMsgRetainByNumHeaders = 3;

  attribute boolean useServerDefaults;
  attribute nsMsgRetainByPreference retainByPreference;
  attribute unsigned long daysToKeepHdrs;
  attribute unsigned long numHeadersToKeep; 

  // this is orthogonal to retaining by age or num headers, i.e., if
  // it's set, we'll only keep NNN unread messages only, or unread
  // messages less than NNN days old.
  attribute boolean keepUnreadMessagesOnly;
  // this is for keeping offline bodies.
  attribute boolean cleanupBodiesByDays;
  attribute unsigned long daysToKeepBodies;
};
that attribute daysToKeepBodies - that's the one you want to set. Does this make
sense?

Comment 5

[Diane Sun]

What I mean identity is that you might have multiple account on the same server,   
identity is the account in my understanding.

I need  one BOOL pref  for "When I create new folders select them for offline 
use"

 and  one BOOL pref  and one INT pref for "Remove messages older than X days"

since it is per account based  on the IMAP server  and daysToKeepBodies  in 
only implemented for news server.

Comment 6

[David :Bienvenu]

we call that an account, not an identity. I'm still going to put it in the imap
incoming server.

Let me try again to say what I said above - days to retain bodies is not
implemented for imap, and shouldn't be in the UI. It's only there for news. In
order to set it for a news server, you create a retention settings object, set
these two vars in the retentions settings object:

 attribute boolean cleanupBodiesByDays;
  attribute unsigned long daysToKeepBodies;

and then set the retention settings object on the incoming server.

Comment 7

[David :Bienvenu]

it looks to me like you should be using the offlineDownload attribute of
nsIIMapIncomingServer.idl for setting the default offline state of new folders.
This attribute has been there forever, so you don't need to wait for me to do
anything in this bug, it turns out. offlineDownload's not the greatest name for
this attribute (sorry) but it's the attribute we use when upgrading from 4.x, so
I don't want to change it.

Comment 8

[Diane Sun]

OK,  I can use the offlineDownload for the BOOL pref.
I still need  INT pref   for  the days  & a BOOL pref for "When I create new 
folders select them for offline use"

Comment 9

[David :Bienvenu]

I'll try again. The bool pref for "When I create new folders select them for
offline use" is the attribute nsIImapIncomingServer::offlineDownload. DO NOT use
offlineDownload for the BOOL pref for "Remove messages older than X days"

For the two "remove messages older than X days" settings (the bool and the int),
you use an nsIMsgRetentionSettings object, and set the following attributes:

 attribute boolean cleanupBodiesByDays;
  attribute unsigned long daysToKeepBodies;


and then set the retention settings on the incoming server.

For example (please forgive me since I'm not a js programmer, but I think it's
roughly like ths)

var retentionSettingsContractID ="@mozilla.org/msgDatabase/retentionSettings;1"

var retentionSettings = Components.classes[retentionSettingsContractID
].createInstance()

retentionSettings.cleanupBodiesByDays = True/False/whatever
retentionSettings.daysToKeepBodies = XXX

incomingServer->retentionSetings = retentionSettings;

Comment 10

[Diane Sun]

 I was understanging that "days to retain bodies is not implemented for imap", 
retentions is only  for news server.  Correct me if I'm wrong.   I know how to 
use these two attributes,  but wanted to confirm  that the functionality has 
been implemented at the backend for imap server.

Comment 11

[David :Bienvenu]

it's only for news - that doesn't change the code at all, however. You only call
it in the case that the incoming server is a news server.

Comment 12

[Diane Sun]

In that case,  I still don't have any attribute to use for Imap  for the "Remove 
messages older than X days"

Comment 13

[David :Bienvenu]

It's not supported in the backend for IMAP at all, so it should NOT be in the UI
at all. Right? You can get and set the settings, and they should be remembered
by the backend, but it won't have any effect as far as actually removing message
bodies is concerned - so it should be removed from the UI. Let me put it another
way- you DO have a place to set them, but the settings don't work in the backend.

Comment 14

[Diane Sun]

Jennifer,  should we remove this from the UI, in the spec. It is useless since 
Imap server has no support for this.

Comment 15

[Diane Sun]

Attachment: Fix for the select for offline only

Comment 16

[jglick]

Remove "Remove messages bodies older than X days" from the screen shot for 
Offline IMAP. Correct?

Comment 17

[Diane Sun]

right

Comment 18

[Diane Sun]

Attachment: Remove the "Remove Msg Body ..." for IMAP

Comment 19

[racham]

We should have been saying server.offlineDownload instead of  
gImapIncomingServer.offlineDownload. Anyway, that infrastructure need to be 
fixed all offline code segments. Please file a separate bug on that one.

r=bhuvan

Comment 20

[(not reading, please use [e-mail] instead)]

will that UI ever been turned on?  if not, remove it instead of commenting it out.

please attach one complete, final patch for me to sr.

Comment 21

[Marek Z. Jeziorek]

Setting target milestone to 0.9.2 (check it in anytime, even before, when the
tree is open for). Per PDT triage.

Comment 22

[Diane Sun]

Attachment: completed diff

Comment 23

[Blake Ross]

      else
         document.getElementById("offline.notDownloadMin").setAttribute
("value", "50");
 
+	if(gServerType == "imap") {
+        gImapIncomingServer = gIncomingServer.QueryInterface
(Components.interfaces.nsIImapIncomingServer);
+        document.getElementById("offline.newFolder").checked =  
gImapIncomingServer.offlineDownload;
+	}

You have two tabs here.

+	if(gServerType == "imap") {
+        gImapIncomingServer.offlineDownload = document.getElementById
("offline.newFolder").checked;
+	}  

You have two tabs here as well.

MailNews is not exempt from the whitespace guidelines followed by the rest of 
the tree.  Please don't contribute to the mess.

Comment 24

[(not reading, please use [e-mail] instead)]

once you fix the tab / whitespace issues, sr=sspitzer

Comment 25

[racham]

This one is fixed along with 76388 (essentially same patch that diane posted here).

Marking fixed.

Comment 26

[Rodney Velasco]

adding myself to Cc.

Comment 27

[Rodney Velasco]

Preference is saved for 
"When I create new folders select them for offline use" 
in the preferences.js file with prefstirng "mail.server.server1.offline_download".
Checkbox is still checked when exit and enter pref settings.
Marking this bug verified on the branch using builds:
Windows: 2001060509
Mac: 2001060408
Linux:2001060411
Still needs to be verified on the trunk, and adding vtrunk keyword.

Comment 28

[grylchan]

Verified using commercial trunk builds
2001061504 -win nt 4.0
2001061508 -linux 2.2, red hat 7.0
2001061508 -mac os 9.0.4

Removing keyword vtrunk