Found in 07-27-branch JA Win build on WinMe-Ja. Steps: 1) Create a profile and set the UI language to en-US (in JA build), or change an existing profile to en-US. 2) Launch, or relaunch browser, according to action in step 1. The menu will appear in English, but the shortcuts will be JA-style. For example, "F" in "File" should be underlined; instead, an underlined "F" appears in parentheses after "File".

QA -> jonrubin

This also happens when applying the DE lang pack.

If I create a ja-JP UI profile in 07-27-branch US build, the shortcuts look correct (JA style); if I switch that profile to English, the shortcuts look correct (EN style). So this problem happens only in JA build.

It is a corebug. The preference of the appending the accesskey is in the language pack (navigator.properties). But it doesn't change when the users switch the language packs. Assigned to ftang.

If I create an en-US in JA build 07-27-branch, the shortcuts look JA-style. But if I open the same profile in US build 07-27-branch, the shortcuts look EN-style.

I am confused, please state clearly what is the expected behavior and what is the actual behavior. jbetak, please help to debug this one.

In en-US profile, it is expected that the letter that corresponds to the KB shortcut will be underlined within the word itself; in ja-JP profile, the letter should appear in parentheses after the word. In the US build, this is working correctly, in the JA build, en-US profiles appear in English, but the shortcuts appear in parentheses after the word; ja-JP profiles always appear correctly. I will attach screenshot examples.

When I say "ja-JP profiles always appear correctly", I mean that they appear correctly regardless of whether they are opened in the JA or US build.

jbetak, please talk to me if you have any question. There is nothing wrong with you code, I think. The problem is when we switch the language pack, it doesn't change according to the preference value stored in navigator.properties: intl.menuitems.alwaysappendacceskeys=true for JA pack. intl.menuitems.alwaysappendacceskeys= for US pack. You may talk to vishy regarding this issue.

rchen, thanks for your comments. As far as I remember the code I wrote for bug listens to the preference intl.menuitems.alwaysappendacceskeys value. In a Japanese build we might be setting it to true and not replacing its value by false in a language pack. I'm downloading the latest commercial bits to verify the contents of navigator.properties in different language packs. Couldn't this be fixed by updating the non-Japanese language packs?

still investigating due to abysmal network performance - at this point it appears to be a problem with incorrect navigator.properties preference setting in individual language packs. I´ll debug it as soon as I have a 6.1 Ja RTM build up.

Changing OS to All, since this happens in Linux also (tested JA 07-26-branch build on Linux RH 7.1-Ja).

Jon, are you still seeing this? I just pulled down a fresh 6.1 JA RTM and I can't reproduce the problem per the steps listed above. Is this possibly platform- dependent? I'm testing on US W2K, have you maybe seen this on JA Windows only?

Juraj, Yes, it looks like this problem occurs only on a JA OS (note that I saw this on JA Linux also). I tried this on Win2K-JA, and the problem occurs; however, I also could not reproduce it on Win2K-US, which is what you tested on.

cc'ing some more i18n folks. Roy - would you know, whether this is typical of Japanese OSes? In another words would you know of a possibility to show underlined instead of appended accelerator keys in say Japanese Windows? I would like to determine the priority and schedule a milestone. If this requires overcoming some OS limitations and is generally within the usual behavior for apps on that platform, I'd push it out a little. Otherwise, we should probably fix this ASAP.

Juraj, I do see underlined keys on the Japanese OS when I install the US build. This problem occurs only for the JA build on the JA OS.

Thanks Jon! I'll boot up in JA Windows and try to debug it there...

Jon, could we look at this together? I'm writing this from JA W2K using 6.1 JA RTM with an English language pack and things still seem to be happy on my end...

Sure.

thanks Jon! I think I finally got it - it seems that both the platform is irrelevant. I thought I was able to repro this using an US build, but it really seems to affect Japanese builds only. One thing we discovered together was that you need to have one profile with with the English language pack activated and one with the Japanese. It seems that the profile manager somehow turns on this global preference, when of the the profiles has the Japanese locale. Pretty strange, when you ask me. Still investigating.

seems like the preference intl.menuitems.alwaysappendacceskeys is being read from the wrong locale. For now, it seems to suffice to change "intl.menuitems.alwaysappendacceskeys=true" to "intl.menuitems.alwaysappendacceskeys=" in the Japanese language pack to address the problem. I'd recommend this solution as a quick workaround/remedy. I'm still trying to understand, why apparently some preference values can be read from the wrong locale. I believe the culprit is in the profile manger. It might take a while to see, how this happens, so reconfirming 0.9.5.

mass change, switching qa contact from jonrubin to ruixu.

ok, I guess I know what happened. In the Ja build, the profile is bootup with JA prefs by default. and when user select a profile, we switch to the new pref. Somehow the global variable which control this behavior is in PRBool nsTextBoxFrame::gAlwaysAppendAccessKey = PR_FALSE; which read in the following code nsTextBoxFrame::Init(nsIPresContext* aPresContext,) Notice that nsTextBoxFrame::Init is called for every TextBoxFrame, regardless it is menu or not. So... some UI displayed in the Profile Manager actually use nsTextFrameBox, and there for nsTextFrameBox::Init get called and the global boolean get setup. now user decide to choose a profile which is en-US, but the pref have already been read in. and there are no code to obsolete the value of that global variable. There are two ways to fix this problem- 1. create a user language switch observer in here to purge the global variable . I think we already have some code like that . tao probably know what is the observer name. 2. delay the reading of the pref till we really need it. This mean we should isolate the pref reading code from Init out into a new method - say InitAccessKeyPref nsTextBoxFrame::InitAccessKeyPref() 164 if (!gAccessKeyPrefInitialized) { 165 nsCOMPtr<nsIPref> prefs = do_GetService(NS_PREF_CONTRACTID); 166 gAccessKeyPrefInitialized = PR_TRUE; 167 if (prefs) { 168 nsXPIDLString prefValue; 169 nsresult res = prefs->GetLocalizedUnicharPref("intl.menuitems.alwaysappendacceskeys", getter_Copies(prefValue)); 170 if (NS_SUCCEEDED(res)) { 171 gAlwaysAppendAccessKey = nsDependentString(prefValue).Equals(NS_LITERAL_STRING("true")); 172 } 173 } 174 } and call it when we really need to access it- which mean add if (!gAccessKeyPrefInitialized) InitAccessKeyPref(); in two places- inside the "if (!mAccessKey.IsEmpty()) {" block of nsTextBoxFrame::UpdateAccessTitle() and the "} else {" block of "nsTextBoxFrame::UpdateAccessIndex()"

these two ways are not exculse. The first one will be needed if we need on the fly language switching later. The second one is always good for delay loading nslocale.dll and speed up startup perf.

move it back to ftang

mark it as assign

add to my working list

try the test patch with debugger. It prove my theroy is right. The Init function got hit before we select the profile. And now the new code won't read in pref untill we select the profile because the current profile code do not have menu. ask for review.

It looks like the origional code is check in by jbeak (loadrunner ). jbetak, please r= this change .

this is a great find Frank! The patch seems reasonable, as it's only delaying the pref read - it was not clear to me back when I worked on this that putting the initialization into nsTextBoxFrame::Init will cause the pref to be read too early. Just pointing this out so you don't catch fire from whoever is doing the sr: tab-width: 4; indent-tabs-mode: nil. r=jbetak

>tab-width: 4; indent-tabs-mode: nil. ???

Comment on attachment 53858 [details] [diff] [review] testing patch. This is just a suggestion, I know this patch probably works fine ... Wouldn't it be better to just add an access method for gAlwaysAppendAccessKey which included the pref init code: PRBool nsTextBoxFrame::AlwaysAppendAccessKey() { < pref init code goes here > return gAlwaysAppendAccessKey; } And then just modify the points that used to access gAlwaysAppendAccessKey directly in the following manner: - if ((mTitle.Find(mAccessKey, PR_TRUE) == kNotFound) || gAlwaysAppendAccessKey) { + if ((mTitle.Find(mAccessKey, PR_TRUE) == kNotFound) || AlwaysAppendAccessKey()) { This insures that the pref is always init'd before the first access of gAlwaysAppendAccessKey's value, and it makes it so that you don't have to worry about adding calls to an InitAccessKeyPref() method everywhere, especially if in the future you needed to retrieve the value in other parts of the code? Why would you want to make this protected? The globals themselves are only accessible from within nsTextBoxFrame? >+protected: >+ void InitAccessKeyPref(); > private: > > CroppingStyle mCropType;

good point. Let me make a better patch

jbeta: can you r= the v2

tao: I think I need to register a observer to observer the "switch UI language" event to fix this. Could you point me to some sample code? What is the topic I should observe ? Currently, the patch is good enough to cover the case that user shutdown the app and relaunch.

frank: since the language switching won't take effect until relaunch, observing the event won't help. Juraj worked on this area a while ago to set locale selection in the prefs for next run. He might know what event to observe. Juraj?

ok. file another bug for that language switching issue. bug 105489. since we don't need to switch locale on the fly right now. Leave that bug there for now. jbetak, can you focus on review of this patch. Thanks :)

looks good to me, r=jbetak the only nit I had was this: + if (NS_SUCCEEDED(res)) + { + gAlwaysAppendAccessKey = nsDependentString(prefValue).Equals( + NS_LITERAL_STRING("true")); + gAccessKeyPrefInitialized = PR_TRUE; + } Wouldn't that be cleaner in some borderline cases, where we don't get to read the pref value? We'd be forced to come back and try again next time around.

tao, ftang, this is hust FYI: turbo mode needs to be exited when switching the app laguage or region and that to the implementation of an event broadcaster - as spearheded by ccarlen: http://lxr.mozilla.org/seamonkey/search?string=locale-selected This might be another way to set gAccessKeyPrefInitialized to PR_FALSE, but I'm not sure how relevant this is for what Frank is trying to accomplish.

>Wouldn't that be cleaner in some borderline cases, where we don't get to read >the pref value? We'd be forced to come back and try again next time around. It think that dramatically slow down the performance if somehow it always fail. kin can you sr= the new patch ?

Comment on attachment 54003 [details] [diff] [review] v2 of the patch sr=kin@netscape.com Just put an empty line between these 2 lines: + PRBool AlwaysAppendAccessKey(); CroppingStyle mCropType;

fixed and check in

Need some matchable language packs for current trunk build. This bug has to be verified when next JA build and language pack are available.