456291 - Imap auto-sync updates folder that have "mail.check_new_mail" pref _off_

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Emre Birol]

Update on folders where check_new_mail _off_ brings unwanted credentials prompts.

Comment 1

[Emre Birol]

There are two prefs look like controlling "check new mail" behavior: 
 i) mail.check_all_imap_folders_for_new
 ii) mail.check_new_mail

Both set to false by default.

When the user uncheck the "Check for new messages every xx minutes" option on "Server Settings" window, TB creates a new pref for this server.

mail.server.<server_name>.check_new_mail
==

bienvenu: 
o What is the purpose of mail.check_all_imap_folders_for_new and how the user changes it? 
o Is it good enough to test mail.<server_name>.check_new_mail before folder updates to fix this behavior?

thx

Comment 2

[Magnus Melin [:mkmelin]]

(In reply to comment #1)
> o What is the purpose of mail.check_all_imap_folders_for_new and how the user
> changes it? 

It makes the mail check look in *all* imap folders for new messages, not just the inbox (mainly useful when using server side filters). Users can change it using the config editor only atm.

Comment 3

[Emre Birol]

Thanks Magnus. This is false by default, does it mean that TB ignores this pref currently?

Comment 4

[David :Bienvenu]

mail.imap.use_status_for_biff, when set to false, will make us select the folders, instead of using status.

But I'm not sure how you can honor the existing prefs and do the auto sync stuff, unless you were to actually honor them, but upgrade new users to the new default.

Comment 5

[Magnus Melin [:mkmelin]]

(In reply to comment #3)
> Thanks Magnus. This is false by default, does it mean that TB ignores this pref
> currently?

Well it doesn't ignore it, it just doesn't check all folders for new if set to false. I have the impression it's fairly common for power users to set it to true though.

Comment 6

[Mark Banner (:standard8)]

We're not using the blocking-thunderbird3.0b1? flags now, we're using a combination of 3 and the target milestone.

Comment 7

[David :Bienvenu]

In a related note, as Magnus commented in another bug, the auto download stuff should be using STATUS to check if there are new messages, instead of calling update folder, since for a lot of servers, STATUS is a lot cheaper. (this is what mail.app does) So it should probably respect the mail.imap.use_status_for_biff pref, which defaults to true.

If the problem here is the unwanted password prompts, then the auto sync code should simply not attempt to update a folder if it doesn't know the server password, which is something we do in several other places already. I'll attach a simple patch.

Comment 8

[David :Bienvenu]

Attachment: proposed fix

If we haven't authenticated against this server at all in the current session, either by loading the inbox on startup, or checking for new mail, then we make auto sync skip it. This will prevent unwanted password prompts...

Comment 9

[David :Bienvenu]

pinging for review

Comment 10

[Emre Birol]

As I understand it this will prevent auto-sync manager to sync the folders that are not explicitly selected by the user, at least this is what I see when I apply the test. 

Can we simple check "mail.<server_name>.check_new_mail" and skip if it is false?

Comment 11

[David :Bienvenu]

no, it should only prevent that until the user has logged on to the server in the current session, not any particular folder. So if they select any folder, or have check for new mail set, then auto sync will see all folders on that server.

Comment 12

[Emre Birol]

Comment on attachment 346479 [details] [diff] [review]
proposed fix

Only downside of this approach is if "Check for new messages at startup" is not checked, auto-sync manager doesn't kick in for this particular account until the user login to the account (explicitly i.e. by selecting one of its folders) or after x minutes elapsed (where x is the value set at "Check for messages every x minutes" option). Whichever happens first.

Comment 13

[David :Bienvenu]

fix checked in.