134745 - mail notification icon in system tray is the wrong icon (yellow triangle)

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[grylchan]

Using commercial trunk build 2002040103 on Win98.

When new mail arrives, instead of a white envelope w/red
flag in the system tray, the icon is a yellow triangle w/exclamation
mark.

I can successfully reproduce on Win98. I tried NT 4.0,
XP, and 2000k and couldn't reproduce. But Jason and Stephen
have seen it on xp and 2k.

Steps:
1.Login to imap acnt
2.Set your prefs to check new mail every 1 minute
3.click ok. (i then quit and restarted to ensure the pref
  was enabled).
4.Send yourself a mail mesg and wait 1 minute:
5.The Mail alert will first pop up, then dissapear.
6.Then mail notification icon will appear in system tray
result: a yellow triangle
expected: a white envelope w/red flag

Kyle do you think it could be a build issue? Maybe icon is
somehow getting corrupted?

see bug 122626 for other descriptions of this happening..

Comment 1

[grylchan]

adding myself as qa. removing olga's name.

Comment 2

[kysmith0 (gone)]

I don't see this on my nt system either. I don't know much about the icons we
use but this could be something strange with the underlying OS component of the
icon. Since this is only seen on win98 that makes me wonder what is different on
that system as compared to the others regarding these types of things. 

Comment 3

[Jason Kersey]

This isn't 98 only, I've seen it on both win2k and winxp.  Much more on win2k,
almost every time.  Could something in our icon be getting stomped on, causing
it to not load properly, and windows having to use a placeholder?

Comment 4

[grylchan]

nominating for nsbeta1 since this would look bad if we
shipped and that it appears to happen on different window
platforms and would confuse users.

I tried installing 4-2 commercial trunk over a old 3-28 trunk
on Win 2k and the icon was correct.

Comment 5

[Blake Ross]

I have also seen this on XP.

Comment 6

[grylchan]

Attachment: screen shot of the triangle icon in system tray

Comment 7

[[e-mail] (gone - use [e-mail] instead)]

adding (yellow triangle) to help people with queries.

Comment 8

[Michael Buckland]

Discussed in Mail News bug meeting.  Decided to ADT1 and plus this bug.

Comment 9

[grylchan]

Anyone who has seen this bug, can you give any concrete steps to
reproduce? Type of Mail acnts/biff settings? New/migrated
profile? How many mail accounts are you using?

I have seen this on NT 4.0 once. I used a migrated profile
w/2 mail acnts. Same biff settings. Added a aol mail acnt
Set the biff setting for that. quit/restarted and sent
a mesg to my aol acnt. But subsequent tries afterwards, I was
unable to produce it.

thanks

Comment 10

[grylchan]

My steps to reproduce. Tested commercial trunk
2002-04-04-11-trunk on Nt 4.0 and 2002040203 on Win2k.

I don't know if the fact that I am using 2 different mail accounts
but both acnts are located on same mail server is important or not.

Steps to reproduce:
1.Create 2 mail acounts (i tried imap/imap or imap/pop)
2. Set check for new mesgs at 1 min & if using pop also
   check Automatically download new mesgs (I also had leave mesgs on server).
3.Save, quit/restart mozilla
4.Login to both mail accounts
5.Compose a mesg from either account and address it both of your mail
  acnts.
6.Send mesg (enter smtp outgoing password if needed)
7.Wait 1 minute (don't do anything)
8.Alert should pop up first, then dissapear
(it should show that both accounts have new mesgs if it doesn't you'll 
 need to repeat step 5 through 7 again)
9.Mail notification icon should pop up 
10.Correct icon (white envelope w/red flag)
11.Now reply to one of those mesgs you just received
   (can do a reply or reply all either way should work)
12.Send mesg and wait 1 minute again
13.Alert should pop up,dissapear
result: Mail notifcation icon is now that yellow triangle
        all subsequent new mail notifications will show that triangle
        regardless of accnt or time.

        You have to quit and restart mozilla to get icon back to normal.

expected: white envelope/red flag

I think when 2 mail acnts receive mail 'at the exact same time',it will 
get confused the next time either account receives another new mesg and
then displays yellow triangle.

Comment 11

[Tuukka Tolvanen (sp3000)]

I see the yellow triangle all the time every time on win98 builds. I have two
pop accounts on different servers, check mail on startup on, quicklaunch off.
Parts of profile are propably migrated (onnce upon a time, and mangled by manual
fixing of alls sorts of past ...incidents :) Everything seems to work besides
that though.

Comment 12

[Jesse Ruderman]

Oh, I didn't realize this was a bug that doesn't affect everyone.  I assumed the
yellow triangle was a temporary placeholder.

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

I don't know if it helps, but I have the yellow triangle here under Win98 SE
with only one mail account (POP3). I am subscribed to several newsgroups
additionally, though. I think this should make clear that this bug does not only
occur with two accounts being checked. It definitely doesn't depend on
Quicklaunch either.

Comment 14

[Scott MacGregor]

Attachment: the fix

This prevents re-entrant calls into the code which shows the animated alert.
This in turn will prevent multiple calls from attempting to modify the icon in
the system tray which I believe is causing the corrupted triangle icon. I
haven't been able to reproduce the bogus icon on my win2k machine but I believe
this patch will fix it, so we'll check it in and try it out on the trunk.

This patch requires the patch in 133130.

Comment 15

[Scott MacGregor]

Marking a dependency since this change needs the change in 133130.

Comment 16

[racham]

Comment on attachment 79314 [details] [diff] [review]
the fix

r=bhuvan

Comment 17

[David :Bienvenu]

Comment on attachment 79314 [details] [diff] [review]
the fix

sr=bienvenu

Comment 18

[scottputterman]

Adding [adt1 rtm] because we could ship beta without this, but adding adt1.0.0
since we have a fix and it looks pretty simple and it would be great to have. 
Please land this on the trunk.

Comment 19

[Scott MacGregor]

this went into the trunk earlier this evening.

Comment 20

[grylchan]

Using commercial trunk 2002-04-16-06-trunk (2002041603) on
NT, 2k, and 2 win98 machines.

I think this is *almost* fixed.

I could not replicate the problem on NT or 2k using my steps
in comment 10. I tried other scenarios and each time mail
notification icon was correct.

But this isn't fixed on win98 (and I assume winME). I tried
2 different computers. I even did a clean install on one computer
(ie only 1 version of mozilla on that computer, deleted registry,
previous builds) And still notification icon comes up as yellow
triangle.

I tried one mail acnt (imap) and set the biff to 1 minute. I quit/restarted,
composed a mesg to myself, sent it and waited 1 minute. I saw the
alert, then yellow triangle for notification icon appeared.

I tried multiple accounts(imap/pop) (w/same biff setting) and icon is still the
yellow triangle. 

Comment 21

[Scott MacGregor]

I figured out why it's happening on win98 still. According to the following:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/platform/CommCtls/winui/icons_5kxa.asp

DestroyIcon should not be called when the icon was loaded using LoadIcon. I am
in fact using LoadIcon.

This comes into play on Win98/WinME because they don't support unicode versions
of ShellNotify. So we first try to use a unicode version, we get an error back
so we destroy the icon the unicode shell struct was using. Then we try to get a
new handle for the icon. I bet win98 then gets grumpy and says the shared
resource is no longer available so it puts up the yellow triangle.

Patch coming up. 

Comment 22

[Scott MacGregor]

Attachment: don't call DestroyIcon

Comment 23

[Navin Gupta]

Comment on attachment 79488 [details] [diff] [review]
don't call DestroyIcon

r=naving

Comment 24

[(not reading, please use [e-mail] instead)]

Comment on attachment 79488 [details] [diff] [review]
don't call DestroyIcon

sr=sspitzer

Comment 25

[Scott MacGregor]

I believe this last patch is going to do the trick. marking fixed. 

Comment 26

[Paul Wyskoczka]

Gary, can you test this fix on the trunk and update the bug.  Thanks.

Comment 27

[[e-mail] (gone - use [e-mail] instead)]

I tested this on Sheela's Windows 98 box, and this still isn't fixed with:

2002-04-17-18 commercial trunk.

I still see the yellow triangle icon.  Sorry, Scott.

Re-opening.

Comment 28

[Jaime Rodriguez, Jr.]

Removing adt1.0.0 nomination, since this bug was reopned on the trunk. [Note:
This is not needed for MachV beta, but for RTM as stated in Status Whiteboard].

Comment 29

[Scott MacGregor]

this might be turbo related. Is everyone who sees this using turbo? Varada just
told me he sees the same thing happening with his turbo icon sometimes too on
win2k. That could explain why all my attempts to fix this problem in mail are
failing =).

Comment 30

[grylchan]

tried commercial build: 2002-04-18-10-trunk on win98 (didn't have
turbo on when I installed it) and still saw the yellow icon..

Comment 31

[Asa Dotzler [:asa]]

Comment on attachment 79488 [details] [diff] [review]
don't call DestroyIcon

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 32

[John Levon]

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

Comment 33

[John Levon]

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

Comment 34

[JWBohrer]

I don't have turbo on and have never seen anything but the yellow icon. Using
build 2002050206

Comment 35

[Gavin Sinclair]

I have always had the warning triangle appear (similar to IE javascript error
warning). 

Still seeing this with 1.0 RC2, 2002051006 on WinME. Don't know if there is any
functionality that is lost - clicking the triangle with left mouse button brings
any open mail/news window to the front. Right-click does nothing, hovering over
the icon displays a tooltip relating to the name of my POP3 account with the
number of new messages. Does not disappear until all messages in the POP3
account are read.

Comment 36

[Gavin Sinclair]

Just to note the yellow warning triangle icon is appearing the same through to
netscape 7.0 pr1 / mozilla 1.0rc2

Wondering if the comment #21 is the whole story - pop-up seems to appear with
the yellow icon there at the same time as the pop-up - yellow icon is not
replacing a proper mail-notification icon. It is not a replacement handle
(appearing when the notification should be destroyed) as far as I can tell - no
envelope is ever apparent. 

Difficult to tell as I have auto-hide for taskbar but have never seen an
envelope icon with new mail notification. Auto-hide couldn't be a problem? It
still tooltips on mouse over the warning triangle - displays number of new
messages - so is not losing functionality when the triangle icon displayed.

Comment 37

[Wolfgang Kronberg]

Gavin: Auto-hide is not the problem. I have never used Auto-hide, but I see the
yellow triangle all the time. I have *once* seen the real envelope, but never
again (I have no idea what was different at that one time).

Comment 38

[scottputterman]

reassigning to ssu.

Comment 39

[varada]

I have tried all of gary's methods and am unable to reproduce this problem. Help
anyone?

Comment 40

[varada]

Nevermind - disregard the previous comment- it is easily reproducible in a Win
ME machine.

Comment 41

[selmer (gone)]

Can we get an ETA for this fix?

Comment 42

[Sean Su]

Attachment: patch v1.0

The problem was that sizeof() was being used to get the length of a WCHAR type
var.  sizeof() just returns bytes, where in this case, WCHAR is 2 bytes. 
Meaning that sizeof() was returning a size value that was twice as much as what
was expected.

This caused memory corruption when calling wcsncpy().

Patch has been tested under Win98 (where it was always reproduceable) and
Win2k.

Comment 43

[scottputterman]

Great job tracking this down.  Let's try to get this reviewed today if possible.

Comment 44

[varada]

Comment on attachment 87442 [details] [diff] [review]
patch v1.0

r=varada;

Comment 45

[Scott MacGregor]

Comment on attachment 87442 [details] [diff] [review]
patch v1.0

Sean nice catch on the problem. I noticed you took out the -1 stuff. the minus
one is to ensure that we leave an extra byte for the null terminator to the
string. Maybe that isn't necessary but that's why it was inthere. Are we
introducing other types of buffer over runs by taking it out? 

Also, could we just call GetToolTipSize() a couple times instead of duplicating
 sizeof(mWideBiffIconData.szTip)/sizeof(mWideBiffIconData.szTip[0])
and
sizeof(mAsciiBiffIconData.szTip) a couple times?

Comment 46

[Sean Su]

I'm pretty sure that we won't be getting into another buffer overrun.  I was
more thinking that if someone is asking for the actual buffer size, we shouln't
be returning (buffer size - 1).  The callee function should be the one being
careful with making sure there's a space for a null character.

I'll update the patch with calls to GetToolTipSize().

Comment 47

[Sean Su]

Attachment: patch v1.1

fixed to call GetToolTipSize() instead of duplicating code.

Comment 48

[Scott MacGregor]

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

sr=mscott

Comment 49

[Sean Su]

patch checked in to trunk only.

Comment 50

[Marek Z. Jeziorek]

can we get verification that the trunk was actually fixed by this patch before
we accept it into the branch?

Comment 51

[Alfonso Martinez]

Tested in win98, build 2002061404.

For the first time I can see the envelop, thank you.

Comment 52

[grylchan]

using commercial trunk 2002061408
on XP, 2k, Me, & 98 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1a) Gecko/20020614

verified that mail notification icon comes up and is displayed
properly. I don't see the yellow triangle.

Tried with imap and pop mail acnts. Tried sending mutliple
mesgs so they arrive in both acnts at the same time and
the icon always appeared. Tooltip appears when you move mouse
over notification icon.

marking as verified on trunk only.

Comment 53

[Paul Wyskoczka]

Marking adt1.0.1+ on behalf of the adt for checkin to the 1.0 branch.  Please
get drivers approval before checking in.

Comment 54

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Regarding mscott's comment 55:  |man strncpy| on my Linux box says that strncpy
never copies more than n characters, but it doesn't null-terminate if it stops
because of the n rather than the null terminator.  Is there any problem with
this string not being null-terminated?  (Or is PL_strncpyz better?)

Comment 55

[Sean Su]

Attachment: patch v1.2 (trunk)

this patch just null terminates the tool tip buffer if a strncpy() call process
has exceeded the destination's buffer size.

Comment 56

[Sean Su]

Attachment: patch v1.2 (branch)

this patch just null terminates the tool tip buffer if a strncpy() call process
has exceeded the destination's buffer size.

Comment 57

[Scott MacGregor]

Comment on attachment 87988 [details] [diff] [review]
patch v1.2 (branch)

Sean, I'm not sure I see how a null byte now gets written after we copy the
tooltip string. It looks like we only write the null byte if the tool tip
string is larger than the buffer:

+    if (wcslen(aToolTipString) >= toolTipBufSize)
+      mWideBiffIconData.szTip[toolTipBufSize - 1] = 0;

Shouldn't we always write the null byte in the last byte of the buffer?

Comment 58

[Scott MacGregor]

Comment on attachment 87988 [details] [diff] [review]
patch v1.2 (branch)

sr=mscott

Sean explained to me that wcsncpy only fails to write a null byte if the string
you are copying from is larger than the destination buffer. So this new patch
writes a null byte in that case.

Comment 59

[Scott MacGregor]

Comment on attachment 87987 [details] [diff] [review]
patch v1.2 (trunk)

sr=mscott

Comment 60

[Sean Su]

the updated patch has been checked in to the trunk.  seeking approval for the
branch again.

Comment 61

[Asa Dotzler [:asa]]

Comment on attachment 87988 [details] [diff] [review]
patch v1.2 (branch)

a=asa (on behalf of drivers) for checkin to the branch.

Comment 62

[Sean Su]

branch patch checked in to the branch.  marking fixed1.0.1

Comment 63

[grylchan]

Using commercial builds:
branch: 2002-06-19-08-1.0/
trunk: 2002-06-19-08-trunk/

on XP, 2k, ME, and 98

verified the mail notification icon that comes up in system
tray is the correct icon and not the yellow triangle.

Tried my steps to reproduce in comment 10 and it works correctly.

Used both imap and pop mail acnts. Tried with turbo mode on & off.
Tried with auto hide on & off for task bar and no problems.

Tooltip for icon and clicking icon in sytem tray works also.

marking verified on branch. 
Changing keyword fixed1.0.1 to verified1.0.1