462995 - Cannot type 2 or more characters from software keyboard

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

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

Attachment: Patch v1.0

This is a regression of bug 420502.

We cannot input 2 or more characters from software keyboard which only calls insertText. (We can input only 1 character.) And then, TSM document initialization is failed, because keydown event is not sent to us before IME events.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

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

I can't reproduce this bug (there isn't enough information here to
tell me how to).  So I haven't tested this patch.  But it does look
reasonable ... though I have a couple of quibbles:

1)

-    mKeyPressHandled = mGeckoChild->DispatchWindowEvent(geckoEvent);
-    mKeyPressSent = YES;
+    PRBool keyPressHandled = mGeckoChild->DispatchWindowEvent(geckoEvent);
+    // If this is not called by key events, we should not change the members
+    // for key events.
+    if (mCurKeyEvent) {
+      mKeyPressHandled = keyPressHandled;
+      mKeyPressSent = YES;
+    }

The comment is hard to understand.  Is the following replacement accurate?

+    // Only record the results of dispatching geckoEvent if we're currently
+    // processing a keyDown event.

2)

   if (!nsTSMManager::IsComposing() && len > 0) {
+    [self initTSMDocument];
     nsQueryContentEvent selection(PR_TRUE, NS_QUERY_SELECTED_TEXT, mGeckoChild);

Is another call to [self initTSMDocument] needed in the following
context (just before another call to [self
sendCompositionEvent:NS_COMPOSITION_START])?

     if (!nsTSMManager::IsComposing()) {
+      [self initTSMDocument];
       [self sendCompositionEvent:NS_COMPOSITION_START];

Comment 2

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

(In reply to comment #1)
> (From update of attachment 346212 [details] [diff] [review])
> I can't reproduce this bug (there isn't enough information here to
> tell me how to).  So I haven't tested this patch.  But it does look
> reasonable ... though I have a couple of quibbles:

I'm sorry, the steps to reproduce are:

1. Install Kotoeri which is Japanese IME.
2. Select "Display Kana palette" (I'm not sure the caption on English environment) in the menu of keyboard layout switching.
3. Type 2 or more characters from the "Kana palette".

> 2)
> 
>    if (!nsTSMManager::IsComposing() && len > 0) {
> +    [self initTSMDocument];
>      nsQueryContentEvent selection(PR_TRUE, NS_QUERY_SELECTED_TEXT,
> mGeckoChild);
> 
> Is another call to [self initTSMDocument] needed in the following
> context (just before another call to [self
> sendCompositionEvent:NS_COMPOSITION_START])?
> 
>      if (!nsTSMManager::IsComposing()) {
> +      [self initTSMDocument];
>        [self sendCompositionEvent:NS_COMPOSITION_START];

Ah, I was thinking we don't need to call from here. But we might call IME related nsIWidget's APIs from XP part of Gecko. Then, this change is needed.

Comment 3

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

Attachment: Patch v2.0

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 346616 [details] [diff] [review]
Patch v2.0

Looks fine to me.

+    // Only record the results of dispatching geckoEvent if we're currentry

currentry -> currently :-)

Comment 5

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

Attachment: Patch v2.0.1

Comment 6

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

Comment on attachment 346672 [details] [diff] [review]
Patch v2.0.1

We should support software keyboard on Mac.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 346672 [details] [diff] [review]
Patch v2.0.1

a=beltzner, hoping that you'll either file litmus tests or some other form of tests

Comment 8

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

landed.