Closed Bug 734034 Opened 13 years ago Closed 12 years ago

invisible directories still used by thunderbird junk mail management (after deferring an account the junk mail target folder still points to the original account, not the deferred-to one)

Categories

(MailNews Core :: Account Manager, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 20.0

People

(Reporter: olivier.rieux, Assigned: aceman)

References

Details

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.56 Safari/535.11 Steps to reproduce: I use varions pop3 accounts. I configured thunderbird to store incoming messages of various accounts to a single "master" account INBOX folder. That is to say, for the "slave" accounts, I did: "tools/account settings/server settings/advanced/when downlowding messages use the following folder: inbox of another account "..."." Unfortunately, the junk folder of one of the "slave" accounts was the target of various "master" and "slave" Junk filters (Tools/account settings/junk mail settings/move new junk mail to:"..."). Actual results: The "slave" accounts folders were no-longer displayed in the folder pannel. However, the junk filters were still storing messages there (which I found out much later). When trying to configure the target of the junk mail filters (Tools/account settings/junk mail settings/move new junk mail to:"...")., the target did no apear in the dialog (due to the fact that it was an hidden folder, I assume) and thunderbird did not accept memorizing a different target. I had to "un-slave" the target account in order to see its junk folder in the settings dialog, and correct my settings. In the meantime, I could not access mail that had been erroneously detected as junk and stored in the hidden junk folder. Expected results: I believe folders should not be hidden (either in the folder pannel and inthe setting dialogs). They should either be left visible, or deleted, with suitable management (automated action or error message) of their content and of settings refering to them.
So you are using a global inbox right ?
Well, I am not sure what you refer to. I did not any any built-in functionality with such a name, but the result indeed is that I have a global inbox with messages from various pop3 servers.
We call "to store incoming messages of various accounts to a single master account INBOX folder" "Global Inbox". At Server Settings of a POP3 account/Advanced button, - Inbox for this server's account <= Ordinal, Non Global Inbox - Global Inbox(Local Folders account) <= Global Inbox type-1 - Inbox for different account <= Global Inbox type-2 (perhaps your case) (note: "Local Folders" can be selected) When an POP3 account is changed from "not use Global Inbox" to "use Global Inbox", following message is shown by Tb. > Deffer Account? > > If you store this account's new mail in a different account's Inbox, > you will no longer be able to access already downloaded e-mail for this account. > If you have mail in this account, please copy it to another account first. > If you have filters that filter mail into this account, you should disable them > or change the destination folder. > If any accounts have special folders in this account (Sent, Drafts, Templates), > you should change them to be in another account. > > [ OK ] [ Cancel ] > Do you still want to store this account's e-mail in a different account? Have you read the message well? Have you executed required actions stated in the message? Although Junk folder, Archives folder, Virtual Folder(Search folder) are not referred in the message, any user selectable folder setting needs to be changed manually. > They should either be left visible, or deleted, with suitable management > (automated action or error message) of their content and of settings refering to them. "Hiding of deffer account" is done by user's request, and is current design/implementation of "Global Inbox". Currently, "automatic setting change upon change to Global Inbox use" is not implemented yet. Because account and his folders is hidden, user has to change folder selection to visible folder at any setting. Above message is request from Tb for manual setting change to user. IIRC, bug for it is already opened. One of reeasons why "not implemente yet" is "Unified Folder" is already implemented. "Unified Folder" is a kind of successor of "Global Inbox".
OK This is clear, I made a mistake. I would have apreciated Thunderbird to let me see and/or modify my wrong settings after I made the mistake. Thank you for your detailed explanation. Regards.
I think the fix for the already broken targets is bug 536768. Preventing the targets to get broken when activating global inbox is the problem here. The other targets (copies, archive, sent) are automatically fixed when an account is deferred to another one. I can look if and why junk target isn't.
Component: Folder and Message Lists → Account Manager
Product: Thunderbird → MailNews Core
QA Contact: folders-message-lists → account-manager
Assignee: nobody → acelists
Confirming and taking.
Blocks: 536768
Status: UNCONFIRMED → NEW
Ever confirmed: true
Severity: normal → major
OS: Windows XP → All
Hardware: x86 → All
Summary: invisible directories still used by thunderbird junk mail management → invisible directories still used by thunderbird junk mail management (after deferring an account the junk mail target folder still points to the original account, not the deferred-to one)
Version: 10 → Trunk
Depends on: 389139
No longer depends on: 389139
Depends on: 389139
No longer blocks: 536768
Depends on: 536768
(In reply to :aceman from comment #6) > I think the fix for the already broken targets is bug 536768. > Preventing the targets to get broken when activating global inbox is the > problem here. And THIS was probably the source of my grief in bug #787573 (Junk Settings not being saved correctly). I always switched new accounts & entities to use Global Inbox when they were created, but perhaps not soon enough to prevent the damage.
Attached patch patch (obsolete) (deleted) — Splinter Review
So this seems to work. It also fixes the loss of selection in the account tree after an account is defered. I am not sure why we need to always pass 'accountValues' to get/setAccountValue. I'd think they should just take 'account' and work out accountValues internally. Or is that some optimization? But it complicates the callers a bit.
Attachment #684958 - Flags: ui-review?(bwinton)
Attachment #684958 - Flags: review?(iann_bugzilla)
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
Attachment #684958 - Attachment is obsolete: true
Attachment #684958 - Flags: ui-review?(bwinton)
Attachment #684958 - Flags: review?(iann_bugzilla)
Attachment #684961 - Flags: ui-review?(bwinton)
Attachment #684961 - Flags: review?(iann_bugzilla)
Status: NEW → ASSIGNED
Comment on attachment 684961 [details] [diff] [review] patch v2 >+ // This is the same sanitization as in am-junk.js. >+ // TODO: Maybe the routines could be merged some day. Would be good to have some sort of helper function, need to pass an object/array in and only pass back those that have changed. >+ let deferredURI = serverSettings.deferredToAccount ? >+ MailServices.accounts.getAccount(serverSettings.deferredToAccount) >+ .incomingServer.serverURI : null; Would let deferredURI = serverSettings.deferredToAccount && MailServices.accounts.getAccount(serverSettings.deferredToAccount) .incomingServer.serverURI; be better? >+ >+ for each (let account in fixIterator(MailServices.accounts.accounts, >+ Components.interfaces.nsIMsgAccount)) { >+ let accountValues = parent.getValueArrayFor(account); >+ let type = parent.getAccountValue(account, accountValues, "server", "type", null, false); >+ if (type == "pop3") { You could inline type here. r- for the moment.
Attachment #684961 - Flags: review?(iann_bugzilla) → review-
Comment on attachment 684961 [details] [diff] [review] patch v2 The text is long enough that I doubt people will read it, but the change makes sense, so I'll say ui-r=me. ;) Thanks, Blake.
Attachment #684961 - Flags: ui-review?(bwinton) → ui-review+
Bwinton, maybe we could split it into some paragraphs? But I am not sure how that is done in this kind of prompt. Maybe with \n in the string?
Flags: needinfo?(bwinton)
Attached patch patch v3 (deleted) — Splinter Review
IanN, I made the shared function now. More comments than code, but at least it does not diverge in the future. Bwinton, the string again :)
Attachment #684961 - Attachment is obsolete: true
Attachment #685276 - Flags: ui-review?(bwinton)
Attachment #685276 - Flags: review?(iann_bugzilla)
Flags: needinfo?(bwinton)
Attached image screenshot of new text (deleted) —
Attachment #685288 - Flags: ui-review?(bwinton)
Attachment #685288 - Flags: review?(iann_bugzilla)
Comment on attachment 685288 [details] screenshot of new text (I'll do the review on the patch itself…)
Attachment #685288 - Flags: ui-review?(bwinton)
Attachment #685288 - Flags: review?(iann_bugzilla)
Comment on attachment 685276 [details] [diff] [review] patch v3 I'm not sure whether we need the localization note or not, but other than that I like how it looks. (I mean, it's still way more text than anyone will read, but it's better than it used to be. ;) ui-r=me! Thanks, Blake.
Attachment #685276 - Flags: ui-review?(bwinton) → ui-review+
(In reply to Blake Winton (:bwinton) from comment #17) > Comment on attachment 685276 [details] [diff] [review] > patch v3 still needing iann review
Flags: needinfo?(iann_bugzilla)
Comment on attachment 685276 [details] [diff] [review] patch v3 >+ let type = parent.getAccountValue(account, accountValues, "server", "type", >+ null, false); >+ // Try to keep this list of account types not having Junk settings >+ // synchronized with the list in AccountManager.js. >+ if (type != "nntp" && type != "rss" && type != "im") { It would be good if there was a way of telling from the account object whether it was capable of having Junk settings or not, but that is outside of scope of this bug. The code looks good and I've tested it as much as I can but I am not a regular user of junk mail management so could do with further testing from someone that is.
Attachment #685276 - Flags: review?(iann_bugzilla) → review+
Flags: needinfo?(iann_bugzilla)
Attachment #685276 - Flags: review?(mkmelin+mozilla)
Comment on attachment 685276 [details] [diff] [review] patch v3 Review of attachment 685276 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. r=mkmelin
Attachment #685276 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: