122351 - Windows 95 doesn't have Shell_NotifyIconW

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[[e-mail]]

I can't use Mozilla mail any more :-(

The turbo icon uses Shell_NotifyIcon which of course is OK on Windows 95.

Comment 1

[Scott MacGregor]

this is not a blocker. Why would this stop you from using the product if it
never even worked for anyone until Monday anyway =). 

We'll take a look in the next milestone. 

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: suggested patch

Can I get a review for this patch?

Comment 3

[Scott MacGregor]

sorry your patch breaks I18N since it no longer allows us to use the wide string
friendly APIs. We'll have to investigate another solution to make this work on
win95 (i.e. only use the ascii versions if the widestring routine is not defined)

Comment 4

[Sheela Ravindran]

change qa contact.

Comment 5

[[e-mail]]

I submitted this as a blocker because it's not just a broken feature but becuase
this stops msgbase.dll from loading which breaks all of Mozilla mail.

If you are able to come up with a workaround which denys me the one feature,
then that remaining problem would be normal.

Comment 6

[Scott MacGregor]

wow, you should have said so! =). Definetly a blocker then. I'll fix this today.

Comment 7

[Scott MacGregor]

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

Comment 8

[Scott MacGregor]

Attachment: a fix which dynamically tests for Shell_NotifyW

This patch should fix the problems for those who are running with an older
version of the shell which doesn't contain wide string support for shell
notify. Basically I dynamically look inside the shell DLL for Shell_NotifyW. If
it's defined then we use the wide string friendly struct along with the
function ptr to Shell_NotifyW. If we can't find that method then we use the non
wide string struct in conjunction with the standard ::Shell_Notify APIs which
all versions of the shell support. 

It's a bit messy since I maintain 2 structs and have several if branches which
depend on whether we are doing wide string or not but that's the only way I
could think to do it.

Comment 9

[Scott MacGregor]

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

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Suggestion: Add a function to that file, like
static BOOL myNotifyIcon(DWORD message, PNOTIFYICONDATAW nid);
This function creates a NOTIFYICONDATA from NOTIFYICONDATAW and uses it to call
Shell_NotifyIcon.

In the constructor, if you can't find Shell_NotifyIconW in the DLL, you set the
function pointer to myNotifyIcon.

This saves you from the two structs and the branches. What's your opinion?

Comment 11

[Scott MacGregor]

Attachment: new cleaner fix

thanks for your suggestion. I still decided to go with having 2 structs around
but your comments made me think to add a level of abstraction to hide the if
clauses from the rest of the class. In this new patch we have the following
abstraction layer:

  void GenericShellNotify(DWORD aMessage);
  void SetToolTipStringOnIconData(const PRUnichar * aToolTipString);
  void DestroyBiffIcon();
  PRUint32 GetToolTipSize();

Comment 12

[racham]

Comment on attachment 67153 [details] [diff] [review]
new cleaner fix

I like the the abstraction you have added. It makes the  code clean and very
easy to manage.

r=bhuvan.

Comment 13

[Scott MacGregor]

Comment on attachment 67153 [details] [diff] [review]
new cleaner fix

had an sr=sspitzer on this

Comment 14

[Scott MacGregor]

fix checked in. Hopefully win95 will be useable in tomorrow's builds. 

Comment 15

[[e-mail]]

I'll be sure to try this tomorrow once I've downloaded a new nightly but in the
meantime I thought I'd be nitpicking and ask why you have this code:
if (mShellNotifyWideChar)
  mUseWideCharBiffIcon = PR_TRUE
and then test if (mUseWideCharBiffIcon) when it would appear that you could
change all those tests to if (mShellNotifyWideChar).

Comment 16

[Sheela Ravindran]

I downloaded a build from this morning (2002-01-31-06) on win98 and winME. I
still don't see the mail notification icon in the system tray.  
I will check this on win95 and update the bug

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Sheela: This bug is for Mailnews not starting because of an undefined symbol;
not for the symbol not appearing.

Comment 18

[Scott MacGregor]

scottip says this fixed the problem on Win NT where we weren't getting the
icons. Christian, how is win95 looking for you? Is the missing symbol gone,
allowing you to run mail again?

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

mscott: it's neil who has the problem, not me. sorry if you got the impression

Comment 20

[Sheela Ravindran]

Found a machine with win95. Real slow one:(
2002-01-31-06 win95 commercial build
I was able to use mail application with turbo on.  I was able send messages. I
also could see the mail notification icon in the system tray.

Scott,
This seemed to have fixed win95.  But winME and win98 still not working.  
verifying this bug for win95

Comment 21

[bobj]

Mail is working for me on Win95 again!

Comment 22

[[e-mail]]

Mail is now working, so I can start filing new bugs on biff icon issues :-)

Comment 23

[Dimitrios]

Icon doesn't appear at all on Win98, build 2002020703. If this bug is fixed,
where is the Win98 equivalent?

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

Dimitros, this bug is for Mailnews not starting on Windows 95, not for a missing
icon on Windows 98.