449021 - Move IMAP IDLE setting to main account settings screen

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[Martin Fleurke]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: 2.0.0.16 (20080708)

Once you have checked an email account for new messages, it keeps being checked regularly, although the checkbox-option "check every xx minutes" in the accounts-settings is not checked.

I'm not sure if you need an other account which has this option checked to be present.

This bug and Bug 352107 are related

Reproducible: Always

Steps to Reproduce:
1. have an account with "check every xx minutes" disabled
[? 1b: maybe?: have an other account with "check ebvery xx minutes" enabled.?]
2. have the account automatically or manually checked for new emails.
Actual Results:  
On regular intervals, the account is checked for new emails

Expected Results:  
no further checking of new emails.

I'm using IMAP mailfolders, but i believe this is unimportant.

This bug is bugging me, because now I often get a "new email"-notification for unimportant emailaccounts, disturbing me in my work.

The bug is present for a very long time, in many (all?) earlier versions of Thunderbird.

Comment 1

[Tuukka Tolvanen (sp3000)]

A few things come to mind:
 * some of these settings don't take effect until tbird is restarted
 * since you're using imap, if the server supports it you'll get messages (for the selected folder on the account or inbox if none selected I believe) immediately without polling using the 'idle' feature (account settings -> [account] -> server settings -> advanced -> idle)

Comment 2

[Martin Fleurke]

Hmm, disabling this option indeed does help. Thunderbird rests quietly.
Changing severity to 'enhancement'. I think there can still be some improvement. I'm not sure what would be the best, but if this 'idle' feature is equal to 'check all the time for new mail' (in effect, not in technique), it is better to remove it from the advanced section, somehow that it becomes clear that that setting (if supported) overrides or interferes with 'check every xx minutes'.

(maybe just by adding some comment somewhere on the screen).

Comment 3

[Steve Simms]

Confirming.  If the IMAP server supports the IDLE command, and it's enabled in Thunderbird, the "Check every xx minutes" box doesn't work as expected.

This could be addressed by moving the IDLE setting to be the next checkbox after the "Check every xx minutes", and renaming it to something like "Keep connection open if server supports the IDLE command" (except shorter).

I'm going to set the severity at minor (from enhancement) since the current setup breaks the user's reasonable expectations.

Comment 4

[Jo Hermans]

If you disabled polling all together, you might be surprised that email is still coming through. I don't think we should change anything on the GUI for that (the IDLE setting is already well hidden for most users), but maybe we want to disable IDLE processing in that case ?

Comment 7

[Wayne Mery (:wsmwk)]

in addition to previously mentioned issues:
- Bug 362941 - With IMAP mail checking disabled but IDLE on, mail is still checked
- bug 334483 - unchecking IMAP "Check this folder for new messages" option doesn't prohibit notifications through IDLE


And from bug 1150411 ...

(In reply to Nikolay Shopik from comment #4)
> I think it describe perfectly. Only small note IDLE on some servers work
> with 60 seconds delay.
> 
> Also if Bug 479133 get implement this means IDLE no longer used but IMAP
> NOTIFY and this is still be real-time notification. But we probably can
> ignore that for now.

Bug 479133 would seem to be a long way off :(

(In reply to rsx11m from comment #5)
> > "get notified of new messages in real time using the IDLE command"
> 
> Especially in light of comment #4, the "using the IDLE command" should
> probably be dropped. Something like "get notified of new messages by the
> server in real time" maybe?

Comment 8

[:aceman]

OK so can we summarize what needs to be done here? Are the "check each xx minutes" and "check realtime using IDLE" options exclusive? Or do they have any reason to be enabled simultaneously?

Comment 9

[Nikolay Shopik]

Yes its exclusive. 

There one thing though, "check each xx minutes" sometimes helps when you lost connection to server and TB still thinks otherwise. So after that period of time it will re-establish connection because of "check each xx minutes" option.

Comment 10

[rsx11m]

(In reply to Nikolay Shopik from comment #9)
> Yes its exclusive. 

Are you sure about that? At least now, they are not and you can check both. I don't see a reason why one shouldn't be allowed to use IDLE as the primary method but poll on a regular basis anyway.

Also, if you go with the radiogroup, you'll have to verify that the server actually supports IDLE (or its successor protocol in the future). The point of the current label is "if the server supports it".

If they are functionally exclusive, checking them both won't be a problem if IDLE prevails and polling overridden other than as a fallback mechanism. Thus, I'd vote for a checkbox.

Comment 11

[Nikolay Shopik]

(In reply to rsx11m from comment #10)
> (In reply to Nikolay Shopik from comment #9)
> > Yes its exclusive. 
> 
> Are you sure about that? At least now, they are not and you can check both.
> I don't see a reason why one shouldn't be allowed to use IDLE as the primary
> method but poll on a regular basis anyway.
> 
> Also, if you go with the radiogroup, you'll have to verify that the server
> actually supports IDLE (or its successor protocol in the future). The point
> of the current label is "if the server supports it".
> 
> If they are functionally exclusive, checking them both won't be a problem if
> IDLE prevails and polling overridden other than as a fallback mechanism.
> Thus, I'd vote for a checkbox.

You are are correct, its not exclusive. Was my misunderstanding. You can do both at same time.

Comment 12

[:aceman]

So the current proposal is to move the "use IDLE" checkbox to the main panel and have options like this:

Check for new messages:
[ ] every [X] minutes
[ ] in real time if the server supports it

Comment 13

[rsx11m]

I don't know if everybody understands what "in real time" means. The important item may be "get notified of new messages by the server" where in this context "Check for new messages:" would be wrong.

[x] Check for new messages at startup
[x] Check for new messages every [__] minutes
[x] Allow server notifications when new messages arrive

Comment 14

[:aceman]

Attachment: patch

So this should be the code part of this. Now decide on the exact wording of the option :)

Comment 15

[:aceman]

Comment on attachment 8609860 [details] [diff] [review]
patch

Review of attachment 8609860 [details] [diff] [review]:
-----------------------------------------------------------------

Let's try this before the string freeze for TB45.

Comment 16

[Richard Marti (:Paenglab)]

Comment on attachment 8609860 [details] [diff] [review]
patch

Review of attachment 8609860 [details] [diff] [review]:
-----------------------------------------------------------------

Not 100% sure because the "if the server supports it" is missing. But this would make the label too long. So ui-r+

::: mailnews/base/prefs/content/am-server.xul
@@ +157,5 @@
>                  label="&loginAtStartup.label;"
>                  accesskey="&loginAtStartup.accesskey;"
>                  prefattribute="value"
>                  prefstring="mail.server.%serverkey%.login_at_startup"/>
> +      </vbox>

When you are touching this, wouldn't you also fix the indentation of the hbox below?

Comment 17

[rsx11m]

(In reply to Richard Marti (:Paenglab) from comment #16)
> Not 100% sure because the "if the server supports it" is missing.

The "Allow" in the new "Allow immediate server notifications..." label kind-of implies that we allow it if the server support it's; if not, it's a don't care. Thus, no ambiguity concerns I'd think.

Comment 18

[:aceman]

(In reply to Richard Marti (:Paenglab) from comment #16)
> When you are touching this, wouldn't you also fix the indentation of the
> hbox below?

The whole groupbox there is messed up. I just put the new element at correct indentation, but all the others are wrong. I can fix the block above mine.

Comment 19

[:aceman]

Attachment: patch v1.1

Comment 20

[Magnus Melin [:mkmelin]]

Comment on attachment 8697872 [details] [diff] [review]
patch v1.1

Review of attachment 8697872 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin with a better accesskey

::: mail/locales/en-US/chrome/messenger/am-server-top.dtd
@@ +23,5 @@
>  <!ENTITY biffStart.label "Check for new messages every ">
>  <!ENTITY biffStart.accesskey "y">
>  <!ENTITY biffEnd.label "minutes">
> +<!ENTITY useIdleNotifications.label "Allow immediate server notifications when new messages arrive">
> +<!ENTITY useIdleNotifications.accesskey "n">

n is taken for User _N_ame

Comment 21

[Ian Neal]

Comment on attachment 8697872 [details] [diff] [review]
patch v1.1

>+++ b/mailnews/base/prefs/content/am-server.xul
>+      </vbox>
>+      <hbox align="center">
>+        <checkbox wsm_persist="true" id="server.doBiff" label="&biffStart.label;"
>+                  accesskey="&biffStart.accesskey;"
>+                  oncommand="onCheckItem('server.biffMinutes', [this.id]);"
>+                  prefattribute="value"
>+                  prefstring="mail.server.%serverkey%.check_new_mail"/>
>+        <textbox wsm_persist="true" id="server.biffMinutes"
>+                 type="number" size="3" min="1" increment="1"
>+                 aria-labelledby="server.doBiff server.biffMinutes biffEnd"
>+                 preftype="int"
>+                 prefstring="mail.server.%serverkey%.check_time"/>
As you are changing the indentation, you might as well follow the one attribute per line style as well.


>+++ b/suite/locales/en-US/chrome/mailnews/pref/am-server-top.dtd
>+<!ENTITY useIdleNotifications.label "Allow immediate server notifications when new messages arrive">
>+<!ENTITY useIdleNotifications.accesskey "n">
As previously mentioned "n" is in use, I think "A" might be available.

Comment 22

[:aceman]

Attachment: patch v1.2

Thanks.

Comment 23

[:aceman]

https://hg.mozilla.org/comm-central/rev/417f2aaf2ed8

Comment 24

[monperrus]

This changed has landed on my EarlyBird TB, thanks :-)

The label is "Allow immediate server notifications when new messages arrive" and does not mention IDLE, which is implicit, esp. for power users.

I suggest to change it into:
"Allow immediate server notifications when new messages arrive (using IMAP IDLE capability)"

Comment 25

[rsx11m]

We are past the string freeze for the new 45.x branch, but (even if not mentioned) I think the intention of the modified label was also to make it more intuitive for the non-power user. The string addition you propose in comment #24 would also make the label unnecessarily long.

On the other hand, we'll need to update the Help content for SeaMonkey anyway, that would be the right place for such implementation details on the more technical level. I can't find a pending SeaMonkey bug on this, thus will open a new one.

Since Thunderbird opted for not using the built-in Help, the details of that checkbox would probably have to be explained in a support.mozilla.org KB article.

Comment 26

[monperrus]

> Since Thunderbird opted for not using the built-in Help, the details of that
checkbox would probably have to be explained in a support.mozilla.org KB
article.

How to do this?

Comment 27

[Charles]

(In reply to martin.monperrus from comment #24)
> This changed has landed on my EarlyBird TB, thanks :-)
> 
> The label is "Allow immediate server notifications when new messages arrive"
> and does not mention IDLE, which is implicit, esp. for power users.

I agree completely, so I opened bug 1313365, proposing to change it to:

"Allow instant server notification of new messages (IDLE support)"

This only adds 3 more characters (5 if you keep the original word 'immediate' instead of 'instant').

I feel strongly that the word IDLE needs to be there to eliminate confusion on the part of those who know what it is. I thought it was but wasn't absolutely sure without a bit of googling and stumbling on this bug.