2036354 - IMAP biff fires repeated new-mail notifications after sleep/wake every ~10s, including e-mails already read on another device

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Max Emig [:maxe]]

Steps to reproduce:

• Have an IMAP account configured on a Windows 11 machine with notifications enabled, IMAP IDLE supported by the server used
• Let the machine go to sleep
• Receive some new e-mails while asleep
• Read some of them on a different device, e.g. Thunderbird for Android
• Wake the machine

Expected results:

• Only one notification bubble/sound gets fired if there are any still unread new e-mails.

Actual results:

• Repeated notification bubbles/sounds every ~10s including e-mails that were already read on another device.

Comment 1

[Max Emig [:maxe]]

Attachment: Bug 2036354 - Skip IMAP biff without a fresh server UNSEEN count. r=#thunderbird-back-end-reviewers

Comment 2

[Max Emig [:maxe]]

We have a pretty minimal patch shape now, it does what I want it to do, but I can't fathom on how to test it.

Comment 3

[Magnus Melin [:mkmelin] (away until Aug 10)]

For testing, this might be of help: https://searchfox.org/comm-central/source/mailnews/base/test/unit/test_newMailNotification.js

Comment 4

[Max Emig [:maxe]]

(In reply to Magnus Melin [:mkmelin] from comment #3)

For testing, this might be of help: https://searchfox.org/comm-central/source/mailnews/base/test/unit/test_newMailNotification.js

Well, the existing Try run proves that the patch does not break any existing tests, but for properly testing it, there are quite a few missing puzzle pieces such as simulating IMAP idle with a reconnect and modifying the read-state in the meantime. Not even the NOOP is functional, just stubbed out and does not ever produce any EXISTS reply we need to hit the changed codepath.

I request a review as-is based on the code and Phabricator comments.

Comment 5

[Max Emig [:maxe]]

We could/should also gate the number of push notifications with sound per minute and set a maximum age of e-mails to i.e. one hour to avoid biff for old e-mails, but that is a design decision and not a logic bug per se.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1944f6133817
Skip IMAP biff without a fresh server UNSEEN count. r=mkmelin

Comment 7

[Max Emig [:maxe]]

I just observed the same symptom again on the latest Daily, but I am still confident that it is much rarer now.

Comment 8

[Max Emig [:maxe]]

Server logs show four IMAP sessions likely racing each other and the repeated biffs, partially about already read messages, were the result of not waiting till its finished.

I still believe in the correctness of the previous fix, but as we all know, improving IMAP without missing edge cases or even causing regressions is a huge undertaking.

Comment 9

[Francesco (no longer active, use PM for contact)]

May I suggest to close this ticked and file a follow-up? This landed in 152, so there won't be more landings here.

Comment 10

[Max Emig [:maxe]]

Sorry for verifying my own bug, new harness, new extensive repro work. Does what it is supposed to, overbiff without, correct amount of biff after, no regression.

Comment 11

[Wayne Mery (:wsmwk)]

Thanks for the fix - this biff activity has been crazy.

If this is an issue older than v152, I'm guessing this will have had duplicate bug reports - has that been checked?