900787 - When scanning for after-a-deadkey expansions, Windows' kbd driver is left in unknown state

Status: UNCONFIRMED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Ilya Zakharevich]

This is a companion to bug https://bugzilla.mozilla.org/show_bug.cgi?id=900777.  (The chunks of code in question are similar, and bugs are similar.)  One of these bugs (I have no clue which one) causes the testable regression mentioned below.  This bug is split from https://bugzilla.mozilla.org/show_bug.cgi?id=880971#c1.

Here I comment on
  KeyboardLayout::GetDeadKeyCombinations()
     http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.cpp#1804

The sad truth is that most of assumptions there are more or less wrong.  This explains at least the second bug in
  http://search.cpan.org/~ilyaz/UI-KeyboardLayout/lib/UI/KeyboardLayout.pm#Firefox_misinterprets_keypresses

=================================================

1046       // Dead-key can pair only with such key that produces exactly one base
1047       // character.

This is wrong.  Dead key will pair with everything.

1057         // Depending on the character the followed the dead-key, the keyboard
1058         // driver can produce one composite character, or a dead-key character
1059         // followed by a second character.

The first part is wrong.  A partially recognized combination may produce up to four
16-bit wcharacters.

The second part is wrong.  An unrecognized combination may produce a dead-key character
followed by up to four 16-bit wcharacters produced by the second keypress.

1074             ret = ::ToUnicodeEx(virtualKey, 0, kbdState, (LPWSTR)baseChars,
1075                                 ArrayLength(baseChars), 0, mKeyboardLayout);
1076             NS_ASSERTION(ret == 1, "One base character expected");
...
1082             deadKeyActive = false;

This assertion is not substantiated.  What follows a dead key is not considered for its “deadness”.  So if it produces a defined combination AFTER a dead key, it still may be a dead key when used standalone (as above).

AFAIU, this causes the second bug in http://search.cpan.org/~ilyaz/UI-KeyboardLayout/lib/UI/KeyboardLayout.pm#Firefox_misinterprets_keypresses

##### The solution: depending on ret, set a correct state of deadKeyActive.
[One needs to call EnsureDeadKeyActive(false).]

Comment 1

[Ilya Zakharevich]

Attachment: diff_scan_after_dead should fix all the issues mentioned in the bug report

Only two LOCs are changed.  The rest fixes comments.

Comment 2

[Ilya Zakharevich]

Actually, in my patch I overlooked one regression: as mentioned,


1852             ret = ::ToUnicodeEx(virtualKey, 0, kbdState, (LPWSTR)baseChars,
1853                                 ArrayLength(baseChars), 0, mKeyboardLayout);
1854             NS_ASSERTION(ret == 1, "One base character expected");
1855             if (ret == 1 && entries < aMaxEntries &&
1856                 AddDeadKeyEntry(baseChars[0], compositeChars[0],
1857                                 aDeadKeyArray, entries)) {
1858               entries++;
1859             }
1860             deadKeyActive = false;

does not make any sense.  ret may be arbitrary!

Comment 3

[Ilya Zakharevich]

Attachment: diff_scan_after_dead should fix all the issues mentioned in the bug report

Untested.

Comment 4

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

Comment on attachment 784743 [details] [diff] [review]
diff_scan_after_dead should fix all the issues mentioned in the bug report

-'ing until tested.