Closed Bug 79239 Opened 23 years ago Closed 23 years ago

Offline UI: Select Folders for Download shouldn't list POP and Local Folders

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: scottputterman, Assigned: sspitzer)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [nsbeta1+][PDT+])

Attachments

(10 files)

Using 2001050704 on Win NT. I know that 4.x used to list local folders but now that we have multiple accounts and treat imported accounts as local folders I don't think we should be showing Local Folders or POP accounts in the Select Folders for Download dialog.
nominating and reassigning
Assignee: bienvenu → dianesun
Keywords: nsbeta1
QA Contact: esther → gchan
From the spec, it only requires not showing the AOL accounts. And 4.x shows local folder. Should we have some consistency between 4.x and 6.x?
I guess I'm not sure what value having folders that can't be selected for offline adds to the dialog. I might be missing something so if anyone knows I don't mind being informed.
I agree it would be nice if POP and Local didn't show in the dialog.
*** Bug 82492 has been marked as a duplicate of this bug. ***
we need to tweak the template rules in msgSelectOffline.xul and fix the folder datasource to pass through that certain folders are not "offline folders". I think we can do this by returning false if the offline level for the folder's server is <= 0. (-1 is undefined, 0 is no support, I think) this would prevent local folders and pop folders from showing up in this dialog. we could then have an boolean attribute on nsIMsgFolder for 'is offline folder" that we could use for disabling / enabling certain ui elements based on the folder pane selection.
OS: Windows NT → All
Hardware: PC → All
taking. this will be all in the core of mailnews and I'd like to do the work. It shouldn't take me long at all, I hope to have something for very early in 0.9.2 so that we can start using this to fix other parts of the UI and cleanup some of our code.
Assignee: dianesun → sspitzer
Target Milestone: --- → mozilla0.9.2
p1, I want to fix this early to unblock monahb and dianesun
No longer blocks: 71128
Status: NEW → ASSIGNED
Priority: -- → P1
to clarify, this bug is about the "select folders" dialog. but to fix that properly, I'm going to do makes some changes to the back end so that we can fix this dialog and the "folder properties" dialog (see #71128) in the correct way.
So, Seth: this work here would also prevent AOL accounts from being shown in the select items dialog, too? Let me know if I need a separate bugscape bug... (hmm, maybe I already logged one, I'll look.)
Whiteboard: [nsbeta1+]
Priority: P1 → P3
adding PDT+
Whiteboard: [nsbeta1+] → [nsbeta1+][PDT+]
> So, Seth: this work here would also prevent AOL accounts from being shown in > the select items dialog, too? Let me know if I need a separate bugscape bug... > (hmm, maybe I already logged one, I'll look.) it should, depending on the offline level we set for those accounts. I'll work with racham to make sure that works.
Fixing this would also fix the folderpane_outliner regression bug 85088, and since this is targeted for 0.9.2 - ain't that just great? :)
fix in hand. the follow fix make it so only servers (and folders) that support offline show up in the dialog. I may need to tweak the threshhold so that AOL imap servers don't show up. this patch also fixes the problem where "Offline Settings" was showing up in account central for pop servers, when it shouldn't, right? I hope to get a review / discuss the fix with racham today.
looks like solid code to me, r=hwaara.
I forgot something. I wanted to add supportsOffline to nsIMsgFolder, because we'll want to use that to enable / disable some offline menu items in the three pane. new patch coming up.
note to laurel / bhuvan: looking at mailnews-ns.js, the default offline level for AOL imap is OFFLINE_SUPPORT_LEVEL_NONE, and the default offline level for netcenter is OFFLINE_SUPPORT_LEVEL_REGULAR those defaults and this patch will make it so AOL folders / servers will not show up in this dialog, but netcenter will. is that desired behaviour?
Yes. AOL should not show, Webmail accounts should.
Yes Seth. As mentioned by laurel also, it is the desired behavior. You know the side effect of this is that we make Offline & Disk Space item disappear in the AccountManager window under pop Accounts. But we do have some disk space settings for POP listed in the panel for this item today. So, there is a need to move those disk space prefs to the pop server panel (am-server.xul) before we land this patch. That is the only thing I am concerned about. Otherwise, all the changes look good. As the matter of fact, we need to move only one pref (of the two that are there). If you look at bug 81494, you will notice that we are trying to keep the pref in offline panel and get rid of the one in server panel. Now because of this change you may have to keep the one in server panel. Then that leaves us with the second pref "Compact folders when it will save over X kb". Diane/Navin, isn't this the one that is moved to the global offline panel ? If so, then we are clean. bhuvan
Small corrections to my last update to make things more clear : * As mentioned by laurel also, it is the desired behavior ==> As mentioned by laurel also, it is the desired behavior for AOL and Webmail accounts. * You know the side effect of this ==> You know the side effect of this patch bhuvan
> You know the side effect of this is that we make Offline & Disk Space item > disappear in the AccountManager window under pop Accounts. But we do have > some disk space settings for POP listed in the panel for this item today. ah, I understand. thanks for catching that. we should not be using the offline level attribute for disk space. since that panel is for both offline AND disk space, I can add another attribute to nsIMsgIncomingServer for supportsDiskSpace. then, in nsMsgAccountManagerDS.cpp (around line 541), I can check supportsDiskSpace in addition to offlineSupportLevel. comments? (I'll work on a new patch while I wait for feedback.)
related comments: 1) in account central, it should say "Offline & Disk Space Settings" under "Advanced Features" for imap, but for pop, it should say "Disk Space Settings". my new patch should allow us to do that (by using the per server supportsDiskSpace attribute. this is bug #83594 2) in account manager, for pop, the panel should be "Disk Space" instead of "Offline & Disk Space". I think my new patch should allow for that fix, too. my new patch is on the way soon. I'll see if I can fix those two problems all in the same patch.
ok, I've got a fix in hand for this bug, and #83594 this patch I'm about to attach makes it so: 1) when selecting offline folders, only folders that support offline show up. 2) under advanced features in "accout central", "offline settings" will not show up for pop (this is bug #83594) 3) in account manager, "offline & diskspace settings" does show up for pop. staying with the spec, I didn't make it so the title was "diskspace" (when there is no offline, like for pop) but I see what it would take. (jglick, if you want this, please update the spec and log a bug on me.) again, staying with the existing spec, I also didn't make it so in account central, under advanced features, it would have "disk space settings" for accounts with disk space settings, but not offline settings (like for pop.) that would have been more work, and again, I'd jglick to update the spec / log a bug if she wants that. here comes the patch, please review again.
You may have to override this function in nsImapIncomingServer to handle AOL as a special case (returning false). Otherwise, the following is going to make AOL accounts to show Offline & Disk Space item in AccountManager. NS_IMETHODIMP +nsMsgIncomingServer::GetSupportsDiskSpace(PRBool *aSupportsDiskSpace) +{ + NS_ENSURE_ARG_POINTER(aSupportsDiskSpace); + *aSupportsDiskSpace = PR_TRUE; + return NS_OK; +} + Let me know if I missed something. bhuvan
good catch. I'll extend the fix so that AOL mail won't show the panel. (the fix will be similar to how the offline level can be overriden per domain.)
once I finish and land, I can clean up the fix to http://bugzilla.mozilla.org/show_bug.cgi?id=79267
OK, to summarize, is this correct? For POP accounts we would move the pref related to disk space on the server setting panel to the "Offline & Disk space" panel to keep the two disk space related prefs together. For IMAP and NNTP accounts "Offline" appears on Account Central but for Pop Accounts it does not. >I didn't make it so the title was "diskspace" (when there is no offline, like >for pop) but I see what it would take. Seth, it would be great if you could do this. I will fill a bug. http://www.mozilla.org/mailnews/specs/accounts/images/AcctOffPop1.gif >I also didn't make it so in account central, under advanced features, it would >have "disk space settings" for accounts with disk space settings, but not >offline settings (like for pop.) This is correct.
Blocks: 17204
> OK, to summarize, is this correct? > > For POP accounts we would move the pref related to disk space on the server > setting panel to the "Offline & Disk space" panel to keep the two disk space > related prefs together. yes. except that it never moved to the server settings panel. is that some other bug? > For IMAP and NNTP accounts "Offline" appears on Account Central but for Pop > Accounts it does not. yes. (except for AOL imap accounts, which do not support offline) > Seth, it would be great if you could do this. I will fill a bug. > http://www.mozilla.org/mailnews/specs/accounts/images/AcctOffPop1.gif ok, no problem. I've got that fix in hand and it will be included in this fix. >>I also didn't make it so in account central, under advanced features, it would >>have "disk space settings" for accounts with disk space settings, but not >>offline settings (like for pop.) > >This is correct. I got confused here. to clarify, there will not be a "Disk Space" link in account central for pop accounts. I'll attach a new patch, and some screen shots.
No longer blocks: 17204
quick question about http://www.mozilla.org/mailnews/specs/accounts/images/AcctOffPop1.gif you have "Disk Space - <acc name>" in the current UI, we don't have "- <acc name>" for any of the panel titles. can you open up a seperate bug on that?
>yes. except that it never moved to the server settings panel. is that some >other bug? sorry, it was originally in Both places. >(except for AOL imap accounts, which do not support offline) right. :-) >to clarify, there will not be a "Disk Space" link in account central for pop >accounts. Correct. Disk Space vs Offline and Disk Space - Bug 85964 >you have "Disk Space - <acc name>" yeah, ignore that for now. separate issue.
"Local Folders", since the folders are on disk, should have a "Disk Space" panel, right? it's easy to fix nsMsgAccountManagerDS.cpp to make "disk space" show up for local folders in the account settings dialog, but am-offline.xul / .js will need some work to support a server of type "none". instead of using server type, we should switch to checking attributes on the server. there is a bug for that already. I'll log a new bug on making "Disk Space" show up for "Local Folders".
the patch fixes the following problems: 1) accounts that don't support offline will not show up in the select folders dialog 2) accounts that don't support offline (like pop) will not have "Offline Settings" in account central 3) accounts that don't support offile, but do support disk space, will have "Disk Space" in account settings. 4) server.supportsDiskSpace and folder.supportsOffline are now available for use to clean up our existing .js
and 5) the offline support level for pop3 servers is properly set at NONE. #17204 is blocked by that.
Blocks: 17204
Patch looks good to me. Good to see the generalization you did in nsImapIncomingServer to create prefnames with host names. r=bhuvan
sr=mscott
a=dbaron for trunk checkin (on behalf of drivers)
fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Using commercial builds: 2001-06-19-11-trunk/ -win nt 4.0 2001-06-19-08-trunk/ mac os 9.0.4 2001-06-18-08-trunk/ linux 2.2, red hat 7.0 Couple of comments and/or questions. 1)In both MAC and Linux builds: I don't see Disk Space or Offline and Disk Space in the 'Account Settings' window (left hand window). 2)For Windows builds: I do see Disk Space in the 'Account Settings' window (left hand window). Is it supposed to display the same GUI as Offline & Disk Space for Imap account? The only thing that is different between the 2 GUI's is the tile bar (imap says Offline & Disk Space) (AOL mail says Disk space). AOL mail's right hand panel for the first pref says "offline" and includes a 'select button') I am assuming Title heading 'Disk Space -<acc name>' didn't make it in this patch? Not sure if I should reopen this bug based on Disk Space not showing up on Mac/Linux builds? Can include screen shots, if needed.
>I am assuming Title heading 'Disk Space -<acc name>' didn't make it in >this patch? Correct. None of the Acct Setting titles are using this yet (so ignore for now). IMAP accounts (regardless of platform) should have "Offline & Disk Space" POP accounts should have "Disk Space". AOL accounts, not sure. We don't do any offline stuff for them currently. Any disk space stuff?
oops should be clearer >1)In both MAC and Linux builds: I don't see Disk Space or Offline > and Disk Space in the 'Account Settings' window (left hand window). This is for AOL accounts only. For AOL accounts I see Server Settings, Copies and Folders, and Addressing. No Disk Space/Offline & DiskSpace prefs for AOL accounts. Again this happens only on Linux/Mac builds. All other accounts are properly displaying Diskspace or Offline & Disk Space. So my question is how AOL accounts are/should be handled? And whether this bug should be reopened or new bugs in bugscape should be filed?
AOL related bugs should be in bugscape.
We already know about AOL bugs and bugscape -- back off. The AOL part of this discussion followed from the overall problem. Since this bug has gotten way too confusing, and since we have some conflicting behavior in the past couple commercial trunk builds regarding the AOL offline display, I logged bugscape bug 6657 to get this nailed down before RTM finalization.
Commercial builds 2001-06-21-09-trunk/ WinNT 4.0 2001-06-21-11-trunk/ Linux 2.2, red hat 7.0 2001-06-21-08-trunk/ MAC 9.0.4 In verifying, I am only going to concern myself with the following: imap, pop, web, local folders, and newsgroups. I will put in comments about aol mail but won't let that deter me from verifying this bug. Verified the following: -In "Select Items for Offline Usage" GUI, the following accounts appear: Imap, web mail, and newsgroups -In Account Central, under Advanced Features: * Offline settings doesn't appear in pop, AOL, and Local folders * Offline settings does appear in Imap, web mail, and newsgroups -In the Account Settings "Tree" (left side window): * Disk Space appears only for POP accounts * Local Folders and AOL mail don't have 'Disk Space' or 'Offline & Disk space' * Offline & Disk Space appears for imap, webmail, and newsgroups The weirdness I saw "For Windows builds: I do see Disk Space in the 'Account Settings' window" is not there anymore. On all platforms, for aol accounts, there is no disk space or offline & disk space setting. Hopefully I covered everything Seth fixed. :-) Marking as verified.
Status: RESOLVED → VERIFIED
Note that bug 83594 had fallen through the cracks. It dealt with offline settings for pop accounts in account central. Since that was fixed & verified in this bug that bug has since been marked verified.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: