User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041423 Minefield/3.0pre Build Identifier: Can't open find toolbar or copy-paste with keyboard shorcuts when using Russian locale Reproducible: Always Steps to Reproduce: 1. Switch to Russian locale 2. Press Ctrl-F Actual Results: Find toolbar doesn't open Expected Results: Find toolbar opens Works in http://hourly-archive.localgho.st/win32/20080414_2052_firefox-3.0pre.en-US.win32.zip Fails in http://hourly-archive.localgho.st/win32/20080414_2241_firefox-3.0pre.en-US.win32.zip http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1208231520&maxdate=1208238059

More blockers piling up on bug 359638.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b4pre) Gecko/2008022702 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008041513 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4) FWIW, With (SeaMonkey and) fr-FR Windows/locale/keyboard, WorksForMe before/after the patch, CapsLock or not: *without Shift: Works. *with Shift: nothing. Then, it doesn't care about min/MAJ, only about Shift key state. I tested the 3: Ctrl+F/C/V.

We need to recover a part of the old code. The old code always converted the charCode to an alphabet or a numeric when Ctrl or Alt is pressed and the pressed key means them. But current code only send the native chars. Therefore, the shortcut keys are broken on Russian, Hebrew... etc. Therefore, if Ctrl or Alt is pressed and shifted char and unshifted char is not alphabet and numeric, then, we should also send the alphabet or numeric for the key. I.e., this patch sends on Russian keyboard: 1. unshifted Cyrillic char and shifted Cyrillic char 2. unshifted Alpharbet/Numeric and shifted Alphabet/Numeric

And the patch also needs this patch: https://bugzilla.mozilla.org/attachment.cgi?id=315912&action=diff The current code has a bug in XP level.

Mozilla/5.0 (X11; U; Linux x86_64; ru; rv:1.9pre) Gecko/2008041611 Minefield/3.0pre All hotkeys (Ctrl-t,Ctrl-w,Ctrl-c,Ctrl-v etc.) are broken in russian locale. Maybe it isn't only Win XP issue?

Comment on attachment 315914 [details] [diff] [review] Patch v1.0 for Win Wait, I need to think for more conditions.

(In reply to comment #5) > Maybe it isn't only Win XP issue? No, This is only on Win32. because I removed the code from Win32's widget.

o.k. this is better. When the key code means an alphabet or a numeric but the unshifted/shifted charCodes are not same it, the user cannot input the alphabet or the numeric. Then, we should append alternate charCodes from the key meanings.

Comment on attachment 315994 [details] [diff] [review] Patch v1.1 + altArray.IsEmpty() ? nsnull : &altArray, Just pass &altArray. I want Karl to check this too.

Comment on attachment 316367 [details] [diff] [review] Patch v1.2 Adding the Latin/Numeric codes is good. + unshiftedChars[0] != DOMKeyCode && shiftedChars[0] != DOMKeyCode) { (In reply to comment #8) > When the key code means an alphabet or a numeric but the unshifted/shifted > charCodes are not same it, the user cannot input the alphabet or the numeric. Do the virtual key codes in wParam get assigned to different hardware keys on non-QWERTY (Latin) layouts (but to QWERTY keys on non-Latin layouts)? r=me if that is the case and with the following changes. + if (aAlternativeCharCodes && !aAlternativeCharCodes->IsEmpty()) + event.alternativeCharCodes.AppendElements(*aAlternativeCharCodes); Remove "&& !aAlternativeCharCodes->IsEmpty()" as it isn't necessary. + nsTArray<nsAlternativeCharCode>* aAlternativeChars, const

(In reply to comment #13) > (From update of attachment 316367 [details] [diff] [review]) > Adding the Latin/Numeric codes is good. > > + unshiftedChars[0] != DOMKeyCode && shiftedChars[0] != DOMKeyCode) > { > > (In reply to comment #8) > > When the key code means an alphabet or a numeric but the unshifted/shifted > > charCodes are not same it, the user cannot input the alphabet or the numeric. > > Do the virtual key codes in wParam get assigned to different hardware keys on > non-QWERTY (Latin) layouts (but to QWERTY keys on non-Latin layouts)? I'm not sure. But If there are such cases, my patch helps to double meanings of a key combination.

Let's take this. This patch fixes this serious regression on some keyboard layouts on Win32, and this is low risky. I will be able to work on bug 429510 after this landing.

(In reply to comment #14) > (In reply to comment #13) > > Do the virtual key codes in wParam get assigned to different hardware keys > > on non-QWERTY (Latin) layouts (but to QWERTY keys on non-Latin layouts)? > > I'm not sure. But If there are such cases, my patch helps to double meanings > of a key combination. We don't want to send QWERTY key codes from AZERTY or Dvorak layouts. If the answer to my question is not "yes", then we need a different test for determining when to send the QWERTY codes. (Perhaps something like the isLatin test you did for GTK.) (That said, the patch as is will not put us in a worse situation than we had before bug 359638 landed.)

virtual PRBool DispatchKeyEvent(PRUint32 aEventType, WORD aCharCode, + nsTArray<nsAlternativeCharCode>* aAlternativeChars, const nsTArray<>*, please?

Drivers: See Comment #15 for this information. This only fixes a nit from the previous patch. Karl: > We don't want to send QWERTY key codes from AZERTY or Dvorak layouts. > > If the answer to my question is not "yes", then we need a different test for > determining when to send the QWERTY codes. (Perhaps something like the isLatin > test you did for GTK.) Don't worry, AZERTY and Dvorak send same keycode as the character, so, we don't send QWERTY layout characters then. I meant that I cannot say "yes" for *all* keyboard layouts and on *all* versions of Windows. But even if there are such strange cases, my patch prefers the inputted ASCII character than the keycode. (the different keycode will not be sent to Gecko.) Note that we sent this code before bug 359638. Sorry for the my unclear comment.

(In reply to comment #18) Sounds good, thanks.

Karl: (In reply to comment #19) > (In reply to comment #18) > Sounds good, thanks. Oops, sorry. I said revert thing: > But even if there are such strange cases, my patch prefers > the inputted ASCII character than the keycode. (the different keycode will not > be sent to Gecko.) Note that we sent this code before bug 359638. my patch prefers the *keycode* than *inputted ASCII characters*. So, the different *keycode* will be sent to Gecko. *However*, we sent this keycode before bug 359638.

(In reply to comment #20) > Oops, sorry. I said revert thing: No problem. I misread the "right" thing. > my patch prefers the *keycode* than *inputted ASCII characters*. So, the > different *keycode* will be sent to Gecko. That sounds good to me.

Comment on attachment 316399 [details] [diff] [review] Patch v1.4 a1.9=beltzner

checked-in.

Ctrl++ (zoom) still doesn't work.

(In reply to comment #26) > Ctrl++ (zoom) still doesn't work. What keyboard layout are you using?

If you meant input language, it's Russian. A layout is a different term.

Just in case you actually meant layout, the one I am using is not what's shipped with Windows. And Ctrl++ worked before this bug got fixed.

(In reply to comment #29) > Just in case you actually meant layout, the one I am using is not what's > shipped with Windows. And Ctrl++ worked before this bug got fixed. The bug depends on the layout. Please file a new bug, and you should explain the your keyboard layout in the new bug.

Note that Ctrl++ works fine for me with Russian standard keyboard layout (Ctrl+=, Ctrl+Shift+=) and Russian typewriter keyboard layout (Ctrl+Shift+|, Ctrl+Shift+!).

Here it is: bug 429898.

Seeing this problem with Hebrew and sm trunk build 2008-04-22 02.