Closed Bug 24594 Opened 21 years ago Closed 4 years ago

Must restart the application for Biff / Check for new messages every x mins to work (or stop working)

Categories

(MailNews Core :: Backend, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 52.0

People

(Reporter: nbaca, Assigned: alta88)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

1.61 KB, patch
alta88
: review+
Details | Diff | Splinter Review
Build 2000012009M13: NT4
Haven't tried Linux or Mac yet.

Overview: For Biff to work I must restart Mail.

Steps to reproduce:
1. Open Mail to a POP or IMAP account
2. Open Account Settings (select Edit|Account Setup)
3. Select the server panel for the POP server
4. Select to "check for new mail ever" 1 "minutes"
5. Click OK to close Account Setup
6. Send a message to the POP account and wait 1 minute

Actual Results: Notice that the Biff icon never appears in the folder pane.
After exiting and restarting Mail then it works as expected.

Expected Results: After selecting to check messages after a certain number of 
minutes, then Bif should work without the need to restart the application.
Setting QA Contact to fenella.
QA Contact: lchiang → fenella
*** Bug 28263 has been marked as a duplicate of this bug. ***
OS: Windows NT → All
Even after restarting, I'm not seeing biff happening.  I have
user_pref("mail.server.server1.check_new_mail", true);
user_pref("mail.server.server1.check_time", 1);
but when I start mail, I wait and wait for new mail to come in, and finally get
suspicious and click GetMsgs, and fifteen messages come in.  Biff just isn't
working reliably.
are you logged on? Right now we don't  biff unless you are logged on.
Yes.  I start the app, I get the imap password dialog and type in my password, I
see my inbox and start reading messages, finish reading messages, and wait ...
and nothing more comes in.

If I manually do GetMsg a few times, then it seems to start biffing by itself,
but I almost never see new mail appear without having to hit GetMsg at least
once in a session first.
I think what akkana mentions was fixed last week by alec.  The bug that Fenella
filed is something that's never been hooked up yet.
Status: NEW → ASSIGNED
Target Milestone: M16
Mail has been biffing fine for me this week, so I think the bug I mentioned has
been fixed.
reassigning to gayatrib.
Assignee: putterman → gayatrib
Status: ASSIGNED → NEW
Blocks: biff
Status: NEW → ASSIGNED
Cleanup work, marking M18.
Target Milestone: M16 → M18
Fenella - can you see if this is working now?
Linux (2000-07-11-08 M17)
Win32 (2000-07-11-09 M17)
Mac (2000-07-11-08 M17)
This problem has been fixed.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Verified it.
Status: RESOLVED → VERIFIED
reopening. I don't think this has been fixed. You still have to restart to get 
changes.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
*** Bug 52369 has been marked as a duplicate of this bug. ***
*** Bug 55852 has been marked as a duplicate of this bug. ***
*** Bug 62305 has been marked as a duplicate of this bug. ***
updating summary.  turning off biff doesn't seem to happen until I restart
either.  yikes.  I think we should bump up the priority on this and fix it for
the next release.
Summary: Must restart the application for Biff to work → Must restart the application for Biff to work (or stop working)
CC-ing self.
*** Bug 60072 has been marked as a duplicate of this bug. ***
reassigning to racham.  marking nsbeta1+ and moving to mozilla0.9
Assignee: gayatrib → racham
Status: REOPENED → NEW
Keywords: nsbeta1
Whiteboard: [nsbeta1+]
Target Milestone: M18 → mozilla0.9
Sheelar is the new QA now
QA Contact: fenella → sheelar
marking nsbeta1- and moving to future milestone.
Keywords: nsbeta1nsbeta1-
Whiteboard: [nsbeta1+] → [nsbeta1+ 2/21]
Target Milestone: mozilla0.9 → Future
*** Bug 60072 has been marked as a duplicate of this bug. ***
2002040203 win32

Biff shows new mail, after mail is read biff doesn't go away.

I'm finding it hard to reproduce.  It seems to be a timing thing related to get
new mail on startup -- when mail is read during the pop-up notification might be
the casue.

Developing...
*** Bug 134803 has been marked as a duplicate of this bug. ***
Summary: Must restart the application for Biff to work (or stop working) → Must restart the application for Biff / Check x mins to work (or stop working)
*** Bug 130781 has been marked as a duplicate of this bug. ***
QA Contact: sheelar → stephend
nominating for nsbeta1
Keywords: nsbeta1
*** Bug 142156 has been marked as a duplicate of this bug. ***
Whiteboard: [nsbeta1+ 2/21] → [nsbeta1+ 2/21][ish]
Bhuvan, I can take this one if you are not working on it.
*** Bug 180305 has been marked as a duplicate of this bug. ***
*** Bug 166477 has been marked as a duplicate of this bug. ***
reassigning to sspitzer
Assignee: racham → sspitzer
Keywords: nsbeta1nsbeta1+
Whiteboard: [nsbeta1+ 2/21][ish] → [ish]
*** Bug 185782 has been marked as a duplicate of this bug. ***
Well now in 2003060409 (OS = Windows XP) I get the reverse effect: I DONT want
it to automatically check for mail but it does anyway
Product: MailNews → Core
*** Bug 267876 has been marked as a duplicate of this bug. ***
*** Bug 137298 has been marked as a duplicate of this bug. ***
Duplicate of this bug: 343140
sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody
Priority: P3 → --
QA Contact: stephend → backend
Hardware: PC → All
Summary: Must restart the application for Biff / Check x mins to work (or stop working) → Must restart the application for Biff / Check for new messages every x mins to work (or stop working)
Whiteboard: [ish]
Target Milestone: Future → ---
Duplicate of this bug: 419852
Product: Core → MailNews Core
I have verified this is still a problem with Thunderbird 2.0.0.17 on Windows XP.
This bug is still existing in Thunderbird 2.0.0.19 on solaris
(In reply to comment #41)
> This bug is still existing in Thunderbird 2.0.0.19 on solaris

Not surprising, since it's still open.
Duplicate of this bug: 233418
Duplicate of this bug: 536359
on bug 536359 there some procedure to reproduce the bug
Blocks: 789883
Duplicate of this bug: 516947
Attached patch biff.patch (obsolete) — Splinter Review
Assignee: nobody → alta88
Attachment #8801201 - Flags: review?(mkmelin+mozilla)
This bug is opened for 16 years. Just a few more and my kids will have a chance to fix it :)
(In reply to Marcin Orlowski from comment #48)
> This bug is opened for 16 years. Just a few more and my kids will have a
> chance to fix it :)

If I had known for certain your kids would fix it, I would have let it ripen some more.. ;)
Comment on attachment 8801201 [details] [diff] [review]
biff.patch


whoever has time first.
Attachment #8801201 - Flags: review?(jorgk)
Comment on attachment 8801201 [details] [diff] [review]
biff.patch

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

As a reviewer you visit code you've never seen before and then you get to ask (silly) questions. First question: What does "biff" stand for?

Comment #0 says that when you change between checks, that's not obeyed without a restart. I can see that nsIMsgIncomingServer.idl has a few attributes, amongst them doBiff and biffMinutes. So the question is: Why don't you do this in SetBiffMinutes()? Another question below.

::: mailnews/base/util/nsMsgIncomingServer.cpp
@@ +1394,5 @@
> +    do_GetService(NS_MSGBIFFMANAGER_CONTRACTID, &rv);
> +  if (NS_SUCCEEDED(rv) && biffService)
> +  {
> +    if (aDoBiff)
> +      (void) biffService->AddServerBiff(this);

Why is it OK to call "add" here? What happens of SetDoBiff(true) is called twice in succession? Or some add-on can do |doBiff = true;| twice. Or are you ignoring an "already added" error here? Shouldn't that be checked explicitly?
(In reply to Jorg K (GMT+2) from comment #51)
> Comment on attachment 8801201 [details] [diff] [review]
> biff.patch
> 
> Review of attachment 8801201 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> As a reviewer you visit code you've never seen before and then you get to
> ask (silly) questions. First question: What does "biff" stand for?
> 

yes, that tradeoff was well considered..  in this context, biff is an automatic poll for new messages from the server.  according to internet lore, it was the name of some unix guy's dog who barked when the mailman came.

> Comment #0 says that when you change between checks, that's not obeyed
> without a restart. I can see that nsIMsgIncomingServer.idl has a few
> attributes, amongst them doBiff and biffMinutes. So the question is: Why
> don't you do this in SetBiffMinutes()? Another question below.
> 

the timer of a switch has nothing to do with with the switch being on or off.

> ::: mailnews/base/util/nsMsgIncomingServer.cpp
> @@ +1394,5 @@
> > +    do_GetService(NS_MSGBIFFMANAGER_CONTRACTID, &rv);
> > +  if (NS_SUCCEEDED(rv) && biffService)
> > +  {
> > +    if (aDoBiff)
> > +      (void) biffService->AddServerBiff(this);
> 
> Why is it OK to call "add" here? What happens of SetDoBiff(true) is called
> twice in succession? Or some add-on can do |doBiff = true;| twice. Or are
> you ignoring an "already added" error here? Shouldn't that be checked
> explicitly?

no, it's explicitly not checked because AddServerBiff does the right thing and ignores servers already in the array.

this is set via the nsIMsgIncomingServer setter in account manager server settings checkbox, on closing the dialog.  the only question is whether a biff should be performed immediately, per reasonable user expectation on turning a switch 'on', or until the biffminutes interval expires.
(In reply to alta88 from comment #52)
> the timer of a switch has nothing to do with with the switch being on or off.
Sorry, I misread comment #0. It's not changing the check interval, it's about switching the checking on.

> no, it's explicitly not checked because AddServerBiff does the right thing
> and ignores servers already in the array.
I don't agree with ignoring the return status here. If you look at nsMsgBiffManager::AddServerBiff() it ignores servers already in the array. There is however a path that could return an error, so it's not OK to ignore it. Equally the return code of nsMsgBiffManager::RemoveServerBiff() should be checked although it currently always returns OK.

> this is set via the nsIMsgIncomingServer setter in account manager server
> settings checkbox, on closing the dialog.  the only question is whether a
> biff should be performed immediately, per reasonable user expectation on
> turning a switch 'on', or until the biffminutes interval expires.
Yes, thanks for explaining this.
In fact, you could add a comment like:
Adding/removing existing/non-existing server is handled without error.
(In reply to Jorg K (GMT+2) from comment #54)
> In fact, you could add a comment like:
> Adding/removing existing/non-existing server is handled without error.

that's all i will do, because it makes no sense to check, for either case.  it is completely ok to ignore it if there's nothing needing to be done about it.  the only other use, on startup, doesn't check either and people have been biffing for 20 years.
(In reply to alta88 from comment #55)
> that's all i will do, because it makes no sense to check, for either case. 
> it is completely ok to ignore it if there's nothing needing to be done about
> it.
OK.

For ignoring the errors, M-C invented a new scheme to be seen here:
https://dxr.mozilla.org/comm-central/source/mozilla/accessible/ipc/other/ProxyAccessible.cpp#26
yeah i know about that, it's not necessary here instead of void, it's for overriding sensitive possibly crashy functions explicitly defined so that callers must get the rv. ie not these functions.
Comment on attachment 8801201 [details] [diff] [review]
biff.patch

Well, Magnus asked me to use |Unused <<| here: Bug 1301706 comment #5 and further. So I'm not sure. If you don't want to check the return value and it's not mandatory to check, why use the |(void)| in the first place?
Attachment #8801201 - Flags: review?(jorgk) → feedback+
I forgot to say: Let's see what Magnus thinks.
well, nsIURI.getSpec is one of the things MOZ_MUST_USE was designed for, and they will or have changed the idl for nsIURI.  again, not the level of these funcs.  void is to avoid warning spew at compile time, and a good practice to show intentional non check.
Comment on attachment 8801201 [details] [diff] [review]
biff.patch

OK then. You're going to add that comment we talked about?
Attachment #8801201 - Flags: review?(mkmelin+mozilla)
Attachment #8801201 - Flags: review+
Attachment #8801201 - Flags: feedback+
Attached patch biff.patchSplinter Review
udpated for comment.
Attachment #8801201 - Attachment is obsolete: true
Attachment #8802676 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/652110fac32ecde08024215d58d45d1c950b69cd
Status: NEW → RESOLVED
Closed: 20 years ago4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
You need to log in before you can comment on or make changes to this bug.