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)
MailNews Core
Account Manager
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 20.0
People
(Reporter: olivier.rieux, Assigned: aceman)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
iannbugzilla
:
review+
mkmelin
:
review+
bwinton
:
ui-review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
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.
Comment 2•13 years ago
|
||
So you are using a global inbox right ?
Reporter | ||
Comment 3•13 years ago
|
||
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.
Comment 4•13 years ago
|
||
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".
Reporter | ||
Comment 5•13 years ago
|
||
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
Confirming and taking.
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
Comment 8•12 years ago
|
||
(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.
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)
Assignee | ||
Comment 10•12 years ago
|
||
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)
Comment 11•12 years ago
|
||
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 12•12 years ago
|
||
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+
Assignee | ||
Comment 13•12 years ago
|
||
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)
Assignee | ||
Comment 14•12 years ago
|
||
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)
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #685288 -
Flags: ui-review?(bwinton)
Attachment #685288 -
Flags: review?(iann_bugzilla)
Comment 16•12 years ago
|
||
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 17•12 years ago
|
||
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+
Comment 18•12 years ago
|
||
(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 19•12 years ago
|
||
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 20•12 years ago
|
||
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
Comment 21•12 years ago
|
||
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.
Description
•