422888 - Cannot change the inputting mode by shortcut keys of AquaSKK without non-needed char inputting

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

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

Attachment: Patch v1.0

AquaSKK is an open source IME of Japanese. That input modes can change L/Shift+L/Ctrl+J/Q/Ctrl+Q. However, we use L/Shift+L/Q, the input mode is changed but the characters are inputted (i.e., l/L/q).

Probably, interpretKeyEvents sent the keyDown events to IME, and IMEs calling insertText for inputting the characters. However, AquaSKK eats the keyDown event if that is above short-cut keys. But then, we fire the non-needed keypress event ourselves. So, after we call interpretKeyEvents but insertText is not called, we should not fire the keypress event if the key combination is a normal character inputting event.

Comment 1

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

Attachment: Patch v1.0 (-u8pw)

Comment 2

[Josh Aas]

Comment on attachment 309371 [details] [diff] [review]
Patch v1.0

+    // If we called interpretKeyEventsCalled and this event is inputting
+    // character event, insertText should be called by interpretKeyEventsCalled.
+    // So, this key event was eaten by IME or something. Then, we should not
+    // fire key press event.

This comment needs to be cleaned up a bit, perhaps like this:

// If we called interpretKeyEvents and this isn't normal character input
// then IME probably ate the event for some reason. We do not want to
// send a key press event in that case.

This is an area of IME I am not very familiar with, have roc check the logic of IsNormalCharInputtingEvent.

Comment 3

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

Attachment: Patch v1.0.1

Thank you Josh.

Only updating the comment. (see attachment 309372 [details] [diff] [review] for -u8pw)

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 310023 [details] [diff] [review]
Patch v1.0.1


>+    // If we called interpretKeyEvents and this isn't normal character input
>+    // then IME probably ate the event for some reason. We do not want to
>+    // send a key press event in that case.
>+    if (!interpretKeyEventsCalled || !IsNormalCharInputtingEvent(geckoEvent)) {
>+      if (mKeyDownHandled)
>+        geckoEvent.flags |= NS_EVENT_FLAG_NO_DEFAULT;

Shouldn't this be || IsNormalCharInp... not !IsNormalChar.. ?

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #0)
> So, after we call interpretKeyEvents but insertText
> is not called, we should not fire the keypress event if the key combination is
> a normal character inputting event.

Oh, I missed this comment..

maybe if (!(interpretKeyEventsCalled && IsNormalCharInputtingEvent)) to clarify the logic, then?

Comment 6

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

Attachment: Patch v1.0.2

Comment 7

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

checked-in, thanks.