257391 - Thunderbird not updating the WinXP registry unread mail counter properly.

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jaye Gallagher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040614 Firefox/0.8
Build Identifier: Thunderbird version 0.7.3 (20040803)

I'm using Thunderbird Version 0.7.3 (20040803) on Windows XP SP1.

I'm wondering whether Thunderbird is correctly updating the Windows XP registry
keys underneath:

HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\UnreadMail

...such that Windows XP is kept aware of the current number of unread emails,
and is thus my laptop is able to turn its email LED on or off as appropriate.

(Details on the above registry key can be found at:
http://support.microsoft.com/default.aspx?scid=kb;en-us;831403
http://support.microsoft.com/default.aspx?kbid=304148 )

Thunderbird seems to be misbehaving, in that it only changes the status of the
Windows XP registry under that point on startup, sometimes on shutdown, and only
occasionally during use. I can't, for instance, reliably set an email message to
"Unread" and have the registry reflect this -- I've been inspecting the registry
values with regedit between such changes, and so am reasonably certain
Thunderbird is doing something amiss.

I've tried setting the "mail.windows_xp_integration.unread_count_interval"
preference to 30 seconds, instead of the default 300, but this makes no difference. 

Futher to this, I've also installed Mozilla 1.7.2 on the same machine, and can
confirm that it does, in fact, handle updating the Windows XP registry correctly
for the unread mail count. This means that this is a bug local to Thunderbird,
and as such, I had a bit of a poke around the source tree.

Alas, I'm not a C++ programmer, but the relevant file seems to be:

/mailnews/base/src/nsMessengerWinIntegration.cpp

...which contains four #ifdef MOZ_THUNDERBIRD sections, all of which seem to be
to do with the process of building the application identifier string that gets
written to the unread mail count section in the Windows XP registry.

A look at what Thunderbird leaves in the registry reveals:

Application;REG_SZ;"" -p -mail
MessageCount;REG_DWORD;0x00000000
Timestamp;REG_BINARY;f0 2d 06 cd 6d 8a c4 01

Now, call me crazy, but I don't think that the value that is being written to
the "Application" key is getting correctly constructed. When Mozilla runs, you
get something like this:

Application;REG_SZ;"C:\Program Files\mozilla.org\Mozilla\mozilla.exe" -p default
-mail

Which seems more correct.

Could it be that since Thunderbird isn't building this application identifier
string correctly in some parts of the code, the registry is only getting updated
by parts of the code some of the time, and inconsistently?

Reproducible: Always
Steps to Reproduce:
1. Start Thunderbird
2. Start regedit, and browse to the appropriate part of the registry mentioned
above.
3. Toggle the unread status of an email message in the Inbox, and refresh the
view of the registry.
4. Observe that the registry isn't updated properly.


Actual Results:  
The registry value for "MessageCount" isn't updated when messages in the
Thunderbird inbox are toggled between read and unread.

Expected Results:  
Thunderbird should be updating the registry value for "MessageCount" every time
the update period (defined by the
mail.windows_xp_integration.unread_count_interval preference) elapses.

Comment 1

[James Davis]

Attachment: Regmon log of UnreadMail behavior

Comment 2

[James Davis]

Comment on attachment 157396 [details]
Regmon log of UnreadMail behavior

01-05 Occured whilst starting up Thunderbird
      Two new message received
      I read one message
06-10 Occured whilst closing down Thunderbird
11-15 Occured whilst starting up Thunderbird
      No new messages received.
      I read the remaining message.
16-20 Occured whilst closing down Thunderbird
21-25 Occured whilst starting up Thunderbird
      No new messages received
      Marked two messages as unread.
26-30 Occured whilst closing down Thunderbird

Comment 3

[James Davis]

Can you log the problems in the behavior whilst using the ntregmon tool from
sysinternals (http://www.sysinternals.com/ntw2k/source/regmon.shtml)? It'll
detail  the changes to the registry with handy timestamps. Also specify your
mail.windows_xp_integration.unread_count_interval setting during the test.

I've given a small example in my attachment and comment in which I explain what
was occuring as the lines in the attachment were logged.

It would also be helpful if you could do this against a more recent nightly
build. I used 20040828.

I don't actually understand the expected behavior, surely it would be easy to
update this registry value each time new mail is fetched or a message remarked
as unread rather than polling every n seconds?

Comment 4

[jwq]

[Following the status whiteboard DUPEME entry]
This bug is a duplicate of Bug 255653 (although I note that Bug 255653 has no
comments, so perhaps Bug 255653 should be marked a duplicate of this bug).

Comment 5

[Howard Chu]

(In reply to comment #3)
> I don't actually understand the expected behavior, surely it would be easy to
> update this registry value each time new mail is fetched or a message remarked
> as unread rather than polling every n seconds?

Yeah, that puzzled me too. Since there is already the biff code to put an alert
in the taskbar whenever new mail is fetched, it would have made sense for the
update to occur right away as well. The polling should only be needed to
discover when to set the registery counter back to zero. I also note that
Microsoft's mailers appear to set the counter totally in realtime; i.e., they
decrement the counter every time you read a message.

Another puzzlement here is that the registry supports separate counters for
individual accounts but Mozilla only updates the registry for your first
account. I have 3 active email accounts, and it's a bit deceptive to only see
the unread mail status for the first account.

Comment 6

[Roland Muller]

Attachment: Patch that removes timer and updates UnreadMail in real time

Comment 7

[Scott MacGregor]

Comment on attachment 160932 [details] [diff] [review]
Patch that removes timer and updates UnreadMail in real time

we don't want to be writing into the registry every time biff goes off.

That's why the timer is there

Comment 8

[Howard Chu]

(In reply to comment #7)
> (From update of attachment 160932 [details] [diff] [review])
> we don't want to be writing into the registry every time biff goes off.
> 
> That's why the timer is there
> 
What in particular is the motivation for the above statement? By default the
mail client only polls for new mail every 10 minutes, so it's not like biff is
going to cause a storm of traffic into the registry.

Comment 9

[James Davis]

(In reply to comment #8)

> What in particular is the motivation for the above statement? By default the
> mail client only polls for new mail every 10 minutes, so it's not like biff is
> going to cause a storm of traffic into the registry.

Even polling for mail at the highest frequency of 1 minute intervals is not
really going to cause a huge number of read/writes to the registry.

Comment 10

[Roland Muller]

(In reply to comment #7)
> (From update of attachment 160932 [details] [diff] [review])
> we don't want to be writing into the registry every time biff goes off.
> 
> That's why the timer is there
> 

No it's not. If you look at the code, you can see that there actually is a write
to the registry each time biff goes off (or at least there should be, hence this
bug :). The timer only resets (or rather should reset) the unread mail count in
case biff hasn't gone off in the timeout period. It's like that in Mozilla mail,
too.

Comment 11

[Howard Chu]

(In reply to comment #10)
> (In reply to comment #7)
> > (From update of attachment 160932 [details] [diff] [review])
> > we don't want to be writing into the registry every time biff goes off.
> > 
> > That's why the timer is there

> No it's not. If you look at the code, you can see that there actually is a write
> to the registry each time biff goes off (or at least there should be, hence this
> bug :). The timer only resets (or rather should reset) the unread mail count in
> case biff hasn't gone off in the timeout period. It's like that in Mozilla mail,
> too.

It looks to me like the patch completely removes the timer. (I haven't actually
compared the patch to the code, so I may be wrong.) But your comment leads me to
believe that the desired behavior is this:
    whenever biff is triggered, that means there is new unread mail. So, write
the unread count into the registry.
    start a timer unconditionally at this point. (i.e., if the counter was
already started, reinitialize it.)
    when the timer goes off, write the current unread count into the registry,
and kill the timer.

So, if new mail arrives, the registry should be updated immediately. On the
assumption that the mail will get read and eventually the unread counter will
decrement to zero, the registry should get updated some time interval later. If
biff fires before the timer, then obviously you have a new count to store.

If this is the behavior that your patch delivers, then I hope Scott reconsiders
his rejection of the patch. If the patch does something else, then I'd like to
see a patch that accomplishes the above.

Comment 12

[Roland Muller]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #7)
> > > (From update of attachment 160932 [details] [diff] [review])
> > > we don't want to be writing into the registry every time biff goes off.
> > > 
> > > That's why the timer is there
> 
> > No it's not. If you look at the code, you can see that there actually is a write
> > to the registry each time biff goes off (or at least there should be, hence this
> > bug :). The timer only resets (or rather should reset) the unread mail count in
> > case biff hasn't gone off in the timeout period. It's like that in Mozilla mail,
> > too.
> 
> It looks to me like the patch completely removes the timer. (I haven't actually
> compared the patch to the code, so I may be wrong.) But your comment leads me to
> believe that the desired behavior is this:
>     whenever biff is triggered, that means there is new unread mail. So, write
> the unread count into the registry.
>     start a timer unconditionally at this point. (i.e., if the counter was
> already started, reinitialize it.)
>     when the timer goes off, write the current unread count into the registry,
> and kill the timer.
> 
> So, if new mail arrives, the registry should be updated immediately. On the
> assumption that the mail will get read and eventually the unread counter will
> decrement to zero, the registry should get updated some time interval later. If
> biff fires before the timer, then obviously you have a new count to store.
> 
> If this is the behavior that your patch delivers, then I hope Scott reconsiders
> his rejection of the patch. If the patch does something else, then I'd like to
> see a patch that accomplishes the above.

Yes, the patch does something else, it disposes of the timer entirely, and for a
reason. It updates the unread mail count in real time (as new mail is coming in,
and as the user marks/unmarks already received mail as (un)read).

My motivations are these:
- The registry key is called UnreadMail for a reason, it should reflect unread
mail, whether it be new mail, or manually marked as such.
- I agree with Howard and James that immediate writes to the registry will not
cause any performance issues due to these writes (although a registry write is a
quite expensive operation, it will not cause any problems when carried out at a
rate of at most once a minute, the only issue here would then be registry
fragmentation), so a timer doesn't seem necessary.

And please consider this: if Thunderbird is to be a serious competitor, then the
WinIntegration module should be considered quite important.

Comment 13

[Jaye Gallagher]

As the original poster of this issue, I really can't understand why nothing
seems to have been done. Perhaps the core of the issue has been lost:
Thunderbird doesn't update the Windows XP registry's unread email counter
properly. Whilst this might seem trivial, it's the sole reason I've not yet
switched -- I work on a laptop that provides an indication of whether unread
email is present using an LED on the front edge. It's visible from a distance,
or when my machine has its lid closed. It's *very* useful functionality and
Thunderbird fails to integrate with it for no reason I can see beyond pointless
quibbling.

Can someone explain exactly *why* this hasn't been fixed yet? At most this would
require a registry update everytime the unread email count in the Inbox changes,
which, even if a user is madly toggling a message between read and unread would
only be one registry update every 200ms. This is trivial. Of course, in regular
use, the registry would only need to be hit around once every 2 minutes or so.

It seems completely silly that such a small thing to fix has prompted such a lot
of debate with no actual result. What's going on?

Comment 14

[Doug Wright]

*** Bug 255653 has been marked as a duplicate of this bug. ***

Comment 15

[Daniel Langsam]

Is there any update on this bug?  I too would be really happy to see it fixed. 
It seems like some of the Thunderbird/Firefox users were having a related
discussion on the thread for bug <a
href='https://bugzilla.mozilla.org/show_bug.cgi?id=246143'>246143</a>, and would
also be interested in seeing it fixed-- Firefox relies on this registry entry to
update it's mail icon.  Is there reluctance to fix it because it's platform
dependent?  If so, are there other ways in which Thunderbird and Firefox can
communicate that might be used by extension writers to at least circumvent the
problem?

Though I don't know the code details, as an end-user I'll echo the opinion that
real-time updates would be best; it seems like it would be worth the small hit
to efficiency that might be incurred.

As an aside, while I could definitely be imagining this, Thunderbird 1.02 seems
a slight bit better about updating the registry; nevertheless, even if I'm not
imagining it, the fundamental problem is still present, as there doesn't seem to
be any overall consistency to the updates, and it's still not real time.

Thanks in advance to anyone who is / will be working on this!

Comment 16

[Mike Cowperthwaite]

Removing the DUPEME (per comment 4 and comment 14).

See also bug 196657, bug 212282, bug 273838.

Comment 17

[Roland Muller]

It's been eight months since I've submitted my clean, simple patch. And my email
LED is glaring at me, with no unread mail.

Please reconsider this bug, it is important to a lot of people. Not only us
laptop users with email LEDs, but XP users with an 'unread mail' message in
their logon screen as well.

If you don't want to use a realtime registry update, at least make the timer
work properly (like Mozilla 1.7.2)

Comment 18

[Howard Chu]

(In reply to comment #17)
> It's been eight months since I've submitted my clean, simple patch. And my email
> LED is glaring at me, with no unread mail.
> 
> Please reconsider this bug, it is important to a lot of people. Not only us
> laptop users with email LEDs, but XP users with an 'unread mail' message in
> their logon screen as well.
> 
> If you don't want to use a realtime registry update, at least make the timer
> work properly (like Mozilla 1.7.2)

This is still bugging me as well. I propose, as a less drastic fix - only do the
realtime registry update if the value changes between zero and non-zero. That's
really all that matters, as far as the user experience is concerned (setting the
Firefox inbox icon, setting the notebook email LED, etc.). As long as the real
value gets set before the mail client exits, then the value in the login screen
will usually be correct. No excessive overhead, no loss of functionality anywhere.

Comment 19

[Howard Chu]

Attachment: More conservative realtime patch

Pretty much the same as the patch in attachment 160932 [details] [diff] [review], but with the additional
check I described in comment #18. The realtime registry update is only
performed when the unread count toggles between zero and non-zero; other
incidental changes really aren't important enough to bother. The count is still
set to the correct value at program startup and shutdown.

Comment 20

[Howard Chu]

Attachment: Fixed timer patch

This patch uses a timer to batch up registry updates. Currently it still uses
the old timer preference, but it probably should just be hardcoded to ~ 20 or
30 seconds, and the old preference deleted. (Note that the default timer
interval is 5 minutes, which is way too slow for this purpose.)

Instead of the repeating timer the old code used (which apparently didn't work)
this code uses a one-shot timer that is started when the first update arrives.
Any further updates that occur between the first update and the timer
expiration are deferred until the timer trips. Also unlike the old code, the
timer object is re-used for subsequent iterations.

Additionally, whenever the unread counter toggles zero / nonzero we do the
update in realtime.

Comment 21

[David :Bienvenu]

Comment on attachment 188697 [details] [diff] [review]
Fixed timer patch

thx for doing this. It looks good. Couple style nits:

+  mUnreadTimerActive = PR_TRUE;
   if (mUnreadCountUpdateTimer) {
     mUnreadCountUpdateTimer->Cancel();
+  } else {

as much as I hate K&R braces, half the file is already in K&R style, so I'll
have to live with that, but can you break up the } else { into two lines?

here, can you put spaces around the '>'?
mLastUnreadCountWrittenToRegistry<1

and for sr, a -uwp style diff would make it easier.

Comment 22

[Howard Chu]

Attachment: Fixed style nits

Comment 23

[Howard Chu]

Comment on attachment 188706 [details] [diff] [review]
Fixed style nits

moving bienvenu's r+ forward

Comment 24

[Howard Chu]

Attachment: Delete timer preference

Timer interval is hardcoded at 20 seconds

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 188713 [details] [diff] [review]
Delete timer preference

Looks good.  Just a couple of minor nits:

>+    if (itemURI && mInboxURI && !nsCRT::strcmp(itemURI, mInboxURI)) {
>+      mCurrentUnreadCount = aNewValue;
>+    }

Prefer strcmp() to nsCRT::strcmp, because compilers and libcs tend to have
specially inlined and/or tuned versions.

>+  mUnreadCountUpdateTimer->InitWithFuncCallback(OnUnreadCountUpdateTimer,
>+    (void*)this, UNREAD_UPDATE_INTERVAL, nsITimer::TYPE_ONE_SHOT);

Prefer C++-style casts (in our case, NS_*_CAST) to C-style casts whenever
possible, as they express more precisely to the compiler what you're trying to
do, which allows it to catch more errors.

sr=dmose, with those nits fixed.

Comment 26

[Howard Chu]

Attachment: Fix strcmp nit

r=bienvenu, sr=dmose replaced nsCRT::strcmp with plain strcmp. Left cast alone,
per IRC discussion.

Comment 27

[Howard Chu]

Attachment: With mailnews.js patch

Oops. Previous patch was missing the diff to mailnews.js.

Comment 28

[Howard Chu]

Comment on attachment 189002 [details] [diff] [review]
With mailnews.js patch

r=bienvenu, sr=dmose, from attachment 188713 [details] [diff] [review]

Comment 29

[David :Bienvenu]

I'll land this soon.

Comment 30

[David :Bienvenu]

fix checked in, thx, Howard.

Comment 31

[Roland Muller]

Great work Howard, thx.

Comment 32

[Worcester12345]

I haven't checked this in a while, but I just today saw that I have 661 new mail messages which is not true. This might need to be reopened. 

Comment 33

[Phil Ringnalda (:philor)]

*** Bug 307493 has been marked as a duplicate of this bug. ***

Comment 34

[Worcester12345]

This still seems broken. (605 new) even though I just received all my email.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060112 Firefox/1.5

version 1.5 (20060111)

Comment 35

[Howard Chu]

(In reply to comment #34)
> This still seems broken. (605 new) even though I just received all my email.
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060112
> Firefox/1.5
> 
> version 1.5 (20060111)
> 

Strange, it works for me in Seamonkey.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060111 SeaMonkey/1.5a Mnenhy/0.7.3.0