429970 - Latin accel keys only function with Shift while in Hebrew layout

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Karl Tomlinson (:karlt)]

Uppercase Latin letters are available on Hebrew keyboards through pressing Shift, but users with Hebrew keyboards I imagine would expect accel keys (Ctrl-F, Ctrl-C, etc) to work without Shift.

http://en.wikipedia.org/wiki/Hebrew_keyboard

I think this is different from the situation of numerals on French keyboards (bug 429219) because Latin letters are normally available both with Shift down and without and have different meanings depending on whether Shift is down.

The solution is probably to loosen this test here:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.cpp&rev=3.740&mark=3287#3282

The GTK code adds the Latin characters unless both shiftedChar and unshiftedChar are Latin (<0xFF).  Maybe that is a suitable heuristic here?
(I guess the best logic for bicameral letters  might actually be different from the best for numerals, but maybe we don't need to get that complicated?)

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: add latin letters unless both the upper and lower case characters are available

I changed my mind:  The VK codes provide information about which key is intended to be used for such an accel key, which is usually the key that can produce such a character.  If the VK code character is not available on that key, then it seems a good indication the the character is not available elsewhere in the layout.

This patch also restores the VK_OEM_PLUS and VK_OEM_MINUS detection (which disappeared in bug 359638) for bug 429898.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 316802 [details] [diff] [review]
add latin letters unless both the upper and lower case characters are available

> +          if (unshiftedChars[0] != DOMKeyCode + 0x20 ||
> +              shiftedChars[0] != DOMKeyCode) {

Maybe, this is wrong. unshiftedChars might have an upper case letter at CapsLocked.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I merged attachment 316802 [details] [diff] [review] to the new patch of bug 429510. And I fixed some issues of attachment 316802 [details] [diff] [review]. We don't have much time, we should fix these bugs at one time.

Comment 4

[Damon Sicore (:damons)]

I'm really concerned that all of these key handling issues are getting mixed up.  Whatever we did to cause this, needs to get un-done or fixed properly.  Now.  

Is there a root cause for all of this?

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> unshiftedChars might have an upper case letter at CapsLocked.

I can't see why this might be uppercase as GetUniCharsWithShiftState is called without setting eCapsLock according to whether CapsLock is down, but maybe it should?

That looks like the cause of this behavior.  (bug 429219 comment 28.)

* ShiftLock+Ctrl+Shift+- has no effect
  ShiftLock+Ctrl+[1-6] switch tabs
  ShiftLock+Ctrl+0 resets size
* ShiftLock+Ctrl+Shift+à resets size

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to comment #4)
> Is there a root cause for all of this?

I think there might be different causes for different bugs.
Some bugs were caused by the removal of some Windows widget code in bug  	359638, and the patch here or now in bug 429510 restores that code.

> Whatever we did to cause this, needs to get un-done or fixed properly. 

I don't think there was ever one time where everything worked as intended, so it's not really possible to just undo any one thing, but I hear the urgency.

Comment 7

[Tony Chung [:tchung]]

we need to add a keyboard navigation litmus suite for these kind of tests

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

fixed by bug 429510.