User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; lt; rv:1.9) Gecko/2008052906 Firefox/3.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; lt; rv:1.9) Gecko/2008052906 Firefox/3.0 Normally, in Windows with Lithuanian keyboard, when you hold AltGr and press 9 or 0, it should type 9 or 0. Now you can see 9 or 0 typed in Lithuanian keyboard only when AltGr is released. It's very inconvenient to type numbers then with Lithuanian keyboard - normally you hold right Alt, and press all the numbers. Now, with this bug, you have to always remember, that you have to release Alt before wanting to press 9 or 0. Reproducible: Always Steps to Reproduce: 1. Go to address bar of Firefox (any form inside any webpage would do). 2. Have Windows keyboard as Lithuanian (also known in Lithuania as Numeric keyboard). 3. Try to hold right Alt (AltGr) and press 9 or 0 - nothing will happen. It should type 9 or 0. Compare it with Windows -> Start -> Run... for example. Actual Results: Nothing happens. Expected Results: 9 or 0 should appear. Typed 9 or 0.

The same problem on Windows XP. It is very serious bug for me, because on notebook computer with Lithuanian keyboard turned on - I'm typing numbers with AltGr key pressed. Because of this bug - I've down-graded my Firefox to previous 2.0.0.16 version, and definitely will not install further Firefox versions until this bug will not be solved. Saulius Maskeliūnas

Confirming this regression on Vista SP1. I see the same problem in an alpha release of Thunderbird 3, thus changing Product/Component to Toolkit/XUL Widgets. This is definately a great annoyance for Notebook users, so should be addressed properly.

BTW there is a workaround for this problem too: the user can install a "Numeric+" keyboard layout from my website: http://rq.lt/numeric/.

I tested on attachment 316954 [details]. Looks like that Ctrl key and Alt key are not consumed in nsKeyboardLayout::GetUnichars().

Yep, and that doesn't surprise me too much. What does surprise me though is that Windows native applications (Start->Run dialog, or Notepad) seem to react just fine and still output those 9 and 0 characters. They don't do that for, e.g., AltGr+u, or AltGr+-...

> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsKeyboardLayout.cpp#156 Maybe, we need to change here: > 144 PRUint32 VirtualKey::GetUniChars (PRUint8 aShiftState, PRUint16* aUniChars, PRUint8* aFinalShiftState) const > 145 { > 146 *aFinalShiftState = aShiftState; > 147 PRUint32 numOfChars = GetNativeUniChars (aShiftState, aUniChars); > 148 > 149 if (aShiftState & (eAlt | eCtrl)) > 150 { > 151 PRUint16 unshiftedChars [5]; > 152 PRUint32 numOfUnshiftedChars = GetNativeUniChars (aShiftState & ~(eAlt | eCtrl), unshiftedChars); > 153 > 154 if (numOfChars) > 155 { > 156 if (!(numOfChars == numOfUnshiftedChars && > 157 memcmp (aUniChars, unshiftedChars, numOfChars * sizeof (PRUint16)) == 0)) > 158 *aFinalShiftState &= ~(eAlt | eCtrl); > 159 } else Looks like the if condition is true on Lithuanian keyboard layout. I think that this is invalid logic.

I think that we can take following ways: 1. If the current keyboard layout has AltGr and it is pressed then, we should consume the key state always. But I'm not sure how to decide whether the keyboard layout has AltGr key. 2. In nsWindow::OnKeyDown, if WM_CHAR/WM_SYSCHAR message are removed from the queue, then, we should consume the key states even if the result shift key state of nsKeyboardLayout::GetUniChars has the key state. Because nsEditor always ignores the keypress event when Alt or Ctrl key is not consumed. I think the 2nd way is safer than 1. But nsKeyboardLayout users must be careful for this issue. Therefore, the 1st way may be strict way.

Or, simply, we should just remove the if statement (#156). The statement already exists the first version of nsKeyboardLayout.cpp.

maybe, we should just remove the condition which compares whether the unshifted chars and the shifted chars are same. Because if the native unichar are not null when Ctrl or Alt key is pressed, the key typing is inputting a (or some) characters to editor. However, if nsWindow does not consume the key states, nsEditor will ignore the keypress event. Therefore, we should always consume the key states.

Just for the reference: If Masayuki is correct, this regression was introduced by fix for Bug 287179: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsKeyboardLayout.cpp&rev=3.14&mark=156#156

Thank you for an interest in a bug that I submitted. I only wanted to mention, that please don't forget that right AltGr is equal to left Alt + left Ctrl. This means, that not only the bug can be reproduced by holding the right AltGr, but also by holding left Alt and left Ctrl together. Alt+Ctrl+9 has to result 9 on the screen just like AltGr+9. A comment, just so that this discussion doesn't go in a wrong direction.

Comment on attachment 338665 [details] [diff] [review] Patch v1.0 This at least doesn't cause any trouble for me, but please maintain the prevalent coding style of the file unless you change the whole file.

o.k. but we should clean-up this file in another bug, the coding style is too different from our rules.

Comment on attachment 339567 [details] [diff] [review] Patch v1.0.1 I agree the style is quite different but it's better to change it all at once without making functional changes at the same time.

Comment on attachment 339567 [details] [diff] [review] Patch v1.0.1 Can you add a test for this?

(In reply to comment #15) > (From update of attachment 339567 [details] [diff] [review]) > Can you add a test for this? o.k. I'll post the test tonight or tomorrow.

Is there any chance to get this into CVS Trunk (3.0 branch) aswell?

(In reply to comment #17) > Is there any chance to get this into CVS Trunk (3.0 branch) aswell? I think if any regressions are not reported on trunk, we can land it to 3.0.

Comment on attachment 339567 [details] [diff] [review] Patch v1.0.1 Oh, this patch has problem. On US and Greek keyboard layout, |numOfChars| is not zero when Alt is pressed but Ctrl is not pressed.

ok, this patch passes all tests.

May I force this a little bit? This bug affects the majority of internet users, it would be a shame not to have it fixed just because it got forgotten, especially when we have a patch handy.

(In reply to comment #21) > This bug affects the majority of internet users, (I forgot to add "in Lithuania", sorry)

It would be simpler and more extensible if synthesizeKey's second parameter was the ID of the element to focus before sending the event.

(In reply to comment #23) > It would be simpler and more extensible if synthesizeKey's second parameter was > the ID of the element to focus before sending the event. ok.

checked-in to trunk. If we will not get regression reports, let's land this to 1.9.0 branch.

(In reply to comment #26) > checked-in to trunk. > > If we will not get regression reports, let's land this to 1.9.0 branch. For how long do we usually wait for them? I'll test a 3.1 branch nightly today to see if the problem is gone.

Thanks Masayuki! The Lithuanian keyboard layout seems to work perfectly with the following build: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081015 Minefield/3.1b2pre Now, how long do we have to wait for a regression before landing that to 1.9.0 branch too?

Basically, I wait one week or more for inputting bugs.

(In reply to comment #29) > Basically, I wait one week or more for inputting bugs. It's been a week already. :)

Masayuki, if there's no regression reported, could we have this patch pushed to 3.0 too? I'm sorry for making pressure, but as we've seen, this issue is really important for some people.

Comment on attachment 342863 [details] [diff] [review] Patch v2.1 O.K. We don't have any regression reports in this week. We should land this patch to 1.9.0 branch for some language users. The patch changes the behavior of dispatching the native keyboard event only when both Ctrl and Alt keys are downed on Windows. So, this patch has impact, but the risk should be low.

Comment on attachment 342863 [details] [diff] [review] Patch v2.1 Too late in 1.9.0.4 for non-security nice-to-haves, we'll get this next time.

This is a safe fix we should take in 1.9.0.5 but it doesn't meet the automatic branch criteria so it's mconnor's call.

Mconnor, what's your call on this?

Comment on attachment 342863 [details] [diff] [review] Patch v2.1 Yeah, let's take it.

Masayuki, please land before midnight tonight, PST.

checked-in.

Verified on: Mozilla/5.0 (Windows; U; Windows NT 6.0; lt; rv:1.9.0.5pre) Gecko/2008112406 GranParadiso/3.0.5pre

(In reply to comment #14) > (From update of attachment 339567 [details] [diff] [review]) > I agree the style is quite different but it's better to change it all at once > without making functional changes at the same time. I filed bug 574340.