Closed Bug 243837 Opened 16 years ago Closed 16 years ago

UI for Global Inbox and deferring storage for pop3 servers to other accounts

Categories

(SeaMonkey :: MailNews: Message Display, defect)

All
Windows 2000
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Details

(Whiteboard: fixed-aviary1.0)

Attachments

(2 files, 5 obsolete files)

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.
Attachment #148692 - Attachment is obsolete: true
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.
Attached patch proposed fix (obsolete) — Splinter Review
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
Attachment #150216 - Flags: superreview?(mscott)
Attachment #150216 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #150216 - Attachment is obsolete: true
Comment on attachment 150216 [details] [diff] [review]
proposed fix

clearing requests.
Attachment #150216 - Flags: superreview?(mscott)
Attachment #150216 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #150299 - Flags: superreview?(sspitzer)
Attachment #150299 - Flags: review?
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+
Attachment #150299 - Attachment is obsolete: true
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.
Attached patch follow-on fix. (obsolete) — Splinter Review
fix based on Christian's suggestion, with the correct variable name...
Attachment #150715 - Flags: superreview?(mscott)
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 #150715 - Attachment is obsolete: true
Attachment #150716 - Flags: superreview?(mscott)
Attachment #150715 - Flags: 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
Whiteboard: fixed-aviary1.0
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!
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.