900777 - When scanning Windows' keyboard for deadkeys, the internal state of keyboard driver is not as expected

Status: NEW

(Core :: Widget: Win32, defect, P4)

Description

[Ilya Zakharevich]

This is a split of (the last paragraph of) https://bugzilla.mozilla.org/show_bug.cgi?id=880971#c3

I continue discussion of the source code in KeyboardLayout.cpp.

The assertion at http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.cpp#1602
          NS_ASSERTION(ret == 2, "Expecting twice repeated dead-key character");
does not make any sense.  What twice-repeated dead key is doing is completely arbitrary (both in theory and in practice).

The assertion above is met only if the dead key does not define what happens when it is pressed twice.  However, in this case what is returned is just the numeric ID of the state of the finite automaton; in many cases this ID converted to a character bears a resemblance to the FUNCTION of the deadkey, but in other cases it is not going to.  Moreover, the information in question IS ALREADY AVAILABLE prior to this call.  The return value -1 from ToUnicode[Ex]() means that one wchar was put into uniChars[0], and this is exactly what deadChar[0] will contain if the assertion holds.

I expect what this code tries to achieve is to find a visual representation of the FUNCTION of the deadkey.  This may be important in some situations (but I do not think it is ever USED by Mozilla!).  To achieve this, I would try the following heuristics:
  1) Try combining with SPACE to get a defined combination;
  2) Try combining with itself to get a defined combination;
  3) if both fail, use the numeric ID returned in uniChars[0].

==============
Moreover, the code in question does not reset the DeadKeyActive state.  One MUST call EnsureDeadKeyActive(false) after doing such ToUnicode[Ex]() calls.

Comment 1

[Ilya Zakharevich]

Attachment: Should fix only the most damaging part of this bug. SetDeadChar() call still assigns junk

Untested; only a partial fix.  Calls EnsureDeadKeyActive() before the definition, but, IIRC, it is defined in some header…

Comment 2

[Ilya Zakharevich]

Either this bug, or its companion bug 900787, cause the second regression mentioned in
   http://search.cpan.org/~ilyaz/UI-KeyboardLayout/lib/UI/KeyboardLayout.pm#Firefox_misinterprets_keypresses
 (The keyboard driver in question is in
   http://k.ilyaz.org/iz
 .)

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 784710 [details] [diff] [review]
Should fix only the most damaging part of this bug.  SetDeadChar() call still assigns junk

-'ing until tested.