Closed Bug 243837 Opened 16 years ago Closed 16 years ago
UI for Global Inbox and deferring storage for pop3 servers to other accounts
In bug 30057, I'm implementing the backend for allowing one pop3 account to defer its storage to another pop3 account, or the local folders directory. If you defer storage, you can also defer get new mail, such that get new mail on the deferred to account will get new mail on the deferred account(s). In the degenerate case, you can defer all your pop3 accounts to the local folders account to get a "Global Pop3 Inbox" like OE has, and when you press get new mail in the local folders account, we download mail from all the pop3 servers that you've configured. We need to design and implement a UI for setting this up. This means both the new account wizard and the mail and news account settings need to be changed to support this. I've done or will do all the other stuff to make the UI work right - the deferred accounts don't show up in the thread pane or the move/copy to menu items, but they show up elsewhere. Get new mail does the right thing, etc. But it would be great if I could get some help with the new account wizard and the account settings dialogs. First, we have to decide what we'll expose in the UI. We can either just have a checkbox that says "Use global Inbox" which makes us defer to the local folders account. Or we can let the user pick which account (pop3 or local folders) account to defer to, if any. In either case, we need a checkbox that says something like "include this account when getting new mail in the deferred to account". Also, when an account defers its storage, it's also implicitly setting the location of the sent/drafts/templates folders to the deferred to account - we need to reflect this in the UI as well. When the user goes into account settings and defers an account that wasn't deferred before, we need to at least warn them that their existing folders won't be accessible, and that they should copy the folders to the deferred to account first. Or, we can offer to do it for them, perhaps create a top-level folder in the deferred to account and copy all the other folders underneath it. If we don't do the copy, we shouldn't delete the original folders. I've hacked up the new account wizard to poke the backend in the appropriate way and I'll attach a patch so you can see what needs to be done to set this up. Thoughts, opinions, offers of help?
This patch shows a couple of the tricks you need to do to set pop3 incoming server attributes from the account wizard, and what attributes you need to poke on the server object itself. One thing I need to do is get it so the deferred account doesn't show up in the folder pane - it's not there when you restart, but it is there immediately after you add it. The problem is that the account gets created before we set it as deferred, and the folder pane doesn't pay attention to the fact that we've deferred it. But we need to create it deferred if possible, so that we don't create the default folders...
David, would it help if we asked some volunteers to test out other apps that do this (like Outlook Express) and post screen shots of what their UI looks like? I can ask on the forums for folks to do research.
If you want to be able to make accounts deferred dynamically then you'll need to generate folder notifications for the deferral changes which the data source can push to the tree builder. (On the subject of data sources, should the folder data source or the account manager data source be handling the deferral state? I find it really annoying that you can't even declare a list of accounts without pulling in the folder data source to keep the names up-to-date.) I can't tell just from looking at the patch whether it does the right thing (whatever that is) for movemail and news accounts. Did someone change the creation order of accounts while I wasn't looking? Only for new profiles local folders inexplicably used to be created after the first account which isn't very useful if you're trying to set up your first account to deliver mail to them. Speaking of which I think the option to "Deliver incoming mail to Inbox on Local Folders" might make more sense to users.
yes, I realize we have to add a notification for the deferral state changing. But that doesn't handle the problem I'm having because the damage is done too early, if that makes sense. News doesn't play in this game... I believe movemail servers should be treated like pop3 servers, but I haven't done anything about that...movemail is synchronous, right? that will make it easier when they are deferred to... I initially tried to put this in the account manager data source, but Seth convinced me that wasn't going to work. Apparently in the folder pane, folders get asked the same question, so it's the folder that has to answer... I like "Deliver incoming mail to Inbox on Local Folders" Research on other UI's might be interesting, but I don't want to deep-end either...
this fixes it so the user only gets asked if they want to use the global inbox, and deferring get new mail is assumed. It also fixes it so we don't create the default folders for a deferred account and show them in the folder pane immediately after going through the account wizard. It's a bit of a hack, using the "valid" server attribute to avoid sending an account loaded notification, but that seems like a reasonable use for it...valid defaults to true for some reason, which is why I have to set it to false explicitly.
Comment on attachment 149158 [details] [diff] [review] second stab at account wizard changes. >+ // we may need local folders before account is "Finished" >+ // if it's a pop3 account which defers to Local Folders. >+ verifyLocalFoldersAccount(gCurrentAccount); [I've always wondered why this is done after the first account is created] >+ var deferStorageBox = document.getElementById("deferStorage"); >+ deferStorageBox.setAttribute("hidden", (serverType == "imap") ? "true" : "false"); XUL elements have this handy boolean hidden property i.e. deferSortageBox.hidden = serverType == "imap"; >+ var deferGetNewMailBox = document.getElementById("deferGetNewMail"); >+ deferGetNewMailBox.setAttribute("hidden", (serverType == "imap") ? "true" : "false"); Didn't you remove this box? >-<!ENTITY accountWizard.size "width: 40em; height: 30em;"> >+<!ENTITY accountWizard.size "width: 40em; height: 35em;"> Do you still need this? The idea of a wizard is to avoid clutter, by keeping the pages small...
thx, locally, I used the hidden attribute and removed the deferGetNewMail code. Re the height increase, yes, I need it - otherwise, scroll-bars show up inside the wizard - very unattractive. Now, I need to figure out how to allow the user to control this in the account settings. The server settings tab for pop3 servers is already quite full. Also, this really needs to be combined with the local directory settings, since if you defer the storage, the local directory setting is not applicable. I think I'd like to expose this feature fully in the account settings UI - so you should be able to choose between: The global inbox (local folders) Some other pop3 or move mail account Your own local directory And if you defer storage, you should be able to also specify that get new mail in the deferred account gets new mail for this account (so people who are setting up dummy accounts can turn this off). Stefan et al, any thoughts on how to arrange this? For pop3 servers, I'm tempted to change the Disk Space pane to something like Message Storage or Disk Storage or something, and remove the local directory setting from the bottom of the server settings pane, and put these settings in the new pane. Or, I could put this in the advanced server settings, and add advanced server settings for pop3. I doubt normal users will want to change this much.
after talking to Seth, I've decided to go this way: by default, for new accounts, we'll use the global inbox. Under the advanced settings for a pop3 server, you can set up your own local directory, or defer to another pop3 server, or use the global inbox (defer to local folders account). Under the advanced settings, you can also specify if you want get new mail in the deferred to account (e.g., local folders), get new mail for this server. I also need to fix import from Outlook and Outlook Express to use the global inbox.
this patch gets everything working in the UI for setting up new pop3 accounts, and editing existing ones, w.r.t. deferring accounts. All the rdf stuff is an attempt to work around problems in the xul template builder - it doesn't add or remove accounts when a property changes that should cause an account to get added or removed. So I changed the mailnews rdf code to pass around the old target in some cases (the xul content builder likes that), and I added a hack to unload and reload the server to get the content to get updated correctly. Some of the UI text I added is quite wordy but I think it's important to explain what's going on.
Attachment #149158 - Attachment is obsolete: true
this patch is the same as the last one, except that it handles creating the of the local folders INBOX if local folders account doesn't have an inbox, when a new pop3 server gets deferred to the global inbox.
Comment on attachment 150216 [details] [diff] [review] proposed fix clearing requests.
Comment on attachment 150299 [details] [diff] [review] cumulative patch, including fix creating of inbox in local folders if first pop3 account is deferred Seth said he's interesting in reviewing this.
I changed the NS_NewAtom calls in nsPop3IncomingServer to be getter_AddRefs(NS_NewAtom) to fix the double ref leak.
Comment on attachment 150299 [details] [diff] [review] cumulative patch, including fix creating of inbox in local folders if first pop3 account is deferred sr=sspitzer can you log a spin off bug about the "CanCreateFoldersOnServer+SupportsOffline" hack? I think there is a bug about switching to a table driven approach, but we don't want to leave that hack in there.
Attachment #150299 - Flags: superreview?(sspitzer) → superreview+
Comment on attachment 150299 [details] [diff] [review] cumulative patch, including fix creating of inbox in local folders if first pop3 account is deferred requesting mscott review - bug 246064 filed for exposing a canDeferTo attribute in the folder ds.
Attachment #150299 - Flags: review? → review?(mscott)
Comment on attachment 150299 [details] [diff] [review] cumulative patch, including fix creating of inbox in local folders if first pop3 account is deferred sweet.
Attachment #150299 - Flags: review?(mscott) → review+
I've played around with this UI just a little, deferring a little-used account to my main account. Looks OK. I dislike it when controls/widgets on a dialog suddenly become visible when a radiobutton is selected; I think disabling them is better UI. This is particularly true on a relatively uncluttered page that's already under "Advanced."
Don't know if I should file a new bug. But since this isn't closed I decided to comment here. The Account Wizard always sets defer_get_new_mail to true - no matter if the checkbox is checked or not. I think getCurrentServerIsDeferred() is wrong and should either be + if (pageData.server && pageData.server.deferStorage && pageData.server.deferStorage.value) + serverDeferred = true; or nicer + if (pageData.server && pageData.server.deferStorage) + serverDeferred = pageData.server.servertype.value;
ah, I'm sure you're right - I switched the default to true, but didn't switch thta code (which only works if the default is false). Patch upcoming.
fix based on Christian's suggestion, with the correct variable name...
Comment on attachment 150715 [details] [diff] [review] follow-on fix. when this feature gets put in the aviary branch, we need this patch too.
Attachment #150715 - Flags: superreview?(mscott) → superreview+
Attachment #150716 - Flags: superreview?(mscott) → superreview+
fixed on trunk, thx Christian.
marking fixed - will open new bugs for new/remaining issues.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Two issues: 1) the dropdown menu for the Get New Messages button still explicitly lists deferred accounts; that doesn't seem desirable, but maybe it is...? 2) there is no context menu item on the Local Folders account (nor Inbox) for Get Messages, when an account is deferred to it.
>1) the dropdown menu for the Get New Messages button still explicitly lists >deferred accounts; that doesn't seem desirable, but maybe it is...? That's desirable and on purpose. Otherwise, there would be no way to get new mail for deferred accounts when you didn't say that you wanted get new mail in the deferred to account to get new mail in the deferred account. >2) there is no context menu item on the Local Folders account (nor Inbox) for >Get Messages, when an account is deferred to it. Good point, I'll file a bug.
when porting to aviary, pick up fix from http://bugzilla.mozilla.org/show_bug.cgi?id=247044
fixed on aviary branch
Another possible issue: if I defer an account to Local Folders, then change my mind and reset it to its own inbox, Local Folders continues to show the Inbox as with a specialized icon, even on restart. I guess I don't expect that folder to be removed, but if nothing's going to it, shouldn't it lose its 'special' status? Other than that, undoing a deferment works very smoothly... nice work!
You need to log in before you can comment on or make changes to this bug.