Simple patch. AppendMenuW is for wide-character strings, which won't work on 9x-based systems. mscott, sr?

Would this affect intl builds, i.e. if the string is not ASCII, would it show up right in the menu? Also similar to LoadImageW vs LoadImage code used in http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#4559 shd we be trying the Wide version first and if it doesnt work, use the non-wide version?

Not that it makes much of a difference, but can you call AppendMenuW() only once and then proceed from there depending on the return status? I'm just thinking that if we have ten items in this menu we'll have nine extra calls to AppendMenuW() and then an additional ten calls to AppendMenu().

Yeah, I considered that. It doesn't matter too much; it's not as if speed is really critical here (especially with only two menuitems), and it would just make it a bit harder to read.

Marking PDT+ as per today's meeting. Blake, in order to take this fix we really need a solution that can be localized on all platforms - Win98 included. Frank - any ideas?

NS_ConvertUCS2toUTF8(openText).get() won't work. We should do the following 1. clone the GetACPString from nsWindow.cpp as below 4502 static char* GetACPString(const nsString& aStr) 4503 { 4504 int acplen = aStr.Length() * 2 + 1; 4505 char * acp = new char[acplen]; 4506 if(acp) 4507 { 4508 int outlen = ::WideCharToMultiByte( CP_ACP, 0, 4509 aStr.get(), aStr.Length(), 4510 acp, acplen, NULL, NULL); 4511 if ( outlen >= 0) 4512 acp[outlen] = '\0'; // null terminate 4513 } 4514 return acp; 4515 } 2. and change the two lines in your patch + ::AppendMenu( mTrayIconMenu, MF_STRING, TURBO_OPEN, NS_ConvertUCS2toUTF8(openText).get() ); + ::AppendMenu( mTrayIconMenu, MF_STRING, TURBO_QUIT, NS_ConvertUCS2toUTF8(exitText).get() ); to the following 10 lines char* openACPText = GetACPString(openText.get()); if (openACPText ) { ::AppendMenu( mTrayIconMenu, MF_STRING, TURBO_OPEN, openACPText ); delete [] openACPText ; } char* quitACPText = GetACPString(quitText.get()); if (quitACPText ) { ::AppendMenu( mTrayIconMenu, MF_STRING, TURBO_QUIT, quitACPText ); delete [] quitACPText ; }

The localized OS does not expect UTF8, they expect charset in ACP. so we need to convert from UCS2 to ACP (ANSI Code Page) but not UTF8.

Frank: thanks! Off to try this now.

Frank, I made one change to your patch; I believe your passing |foo.get()| to GetACPString was a typo, since it takes an nsString. This certainly works fine for me on Windows 2000; can you get someone to test on Windows 9x with i18n characters? Naoki?

so you're not going to use the Wide version ever? this is very expensive on windows nt because you're converting from Wide to Narrow to Wide. *bad*. scc does the ACP stuff look ok? [could the function take an nsAReadableString ?] fwiw there are examples in extra.c which might be useful (although i'm starting to dislike some of its code...) if (hLib = LoadLibraryEx("user32.dll", NULL, LOAD_WITH_ALTERED_SEARCH_PATH))) if (GetProcAddress(hLib, "AppendMenuW")) else ...

Speed is not a huge issue here. We're not trying to make the world's fastest popup menu with two items; we're looking for the lowest risk patch. Yes, we could attempt to do wide-character string appends first if you think it actually matters.

Let's skin the menu -- just kidding!!! I don't think we're going to run into speed issues. As I alluded to in my 2001-07-12 comment, I think that if we add a ton more items to the menu we should maybe re-visit the order that things are done in. But for two items, this seems fine.

considering the number of bugs asking for more menu items in the system tray i think we should address that pretty soon, a quick fix that supports Wide and w95osr0 should be implemented... if you need help and i'm awake, pageme.

I think Frank's suggestion was for the case if (::GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) When AppendMenuW is available, that should be used.

I can apply the patch and change the property file and find machine to test. How can I enable the feature? If I just lanch, the tray icon does not show up.

Naoki: run netscp6.exe -turbo

The patch doesn't compile with the trunk. My branch build is not up to date and it won't show the tray even on NT. I am pulling the branch now.

I pulled the branch but it crashes when I launch with -turbo option. Please change profileManager.properties as below and put the build somewhere so we can test. startButton=Start\u3042 %brandShortName% exitButton=Exit\u3044

where does it crash?

When I have multiple profiles, it asserts at the code below in nsAppRunner.cpp and quits on my branch debug build. I don't see it with commercial release build. // If we are in server mode, profile mgr cannot show UI rv = profileMgr->StartupWithArgs(cmdLineArgs, !IsAppInServerMode()); NS_ASSERTION(NS_SUCCEEDED(rv), "StartupWithArgs failed\n"); if (NS_FAILED(rv)) return rv;

The patch is working fine on Win98 JA with localized strings (see the screen shot) as long as there is only have one profile on the machine.

Yeah, the assertion is expected; that's what happens when you have multiple profiles. Looking for an r= on this patch.

alright. sr=ben@netscape.com

Fix checked in.

Reopening, I forgot to checkin on the trunk. Removing PDT+ so this doesn't show up on radar, but please remember to verify this on win98 Paul.

adding vbranch keyword to make sure this shows up on paw's list of branch bugs to QA.

nav triage: Blake could you check this in on the trunk and close the bug! thanks!

Fixed on trunk.