Closed Bug 462995 Opened 13 years ago Closed 13 years ago

Cannot type 2 or more characters from software keyboard

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Attached patch Patch v1.0 (obsolete) — Splinter Review
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.
Attachment #346212 - Flags: review?(joshmoz)
Attachment #346212 - Flags: review?(smichaud)
Attachment #346212 - Flags: review?(joshmoz)
Attachment #346212 - Flags: review+
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];
(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.
Attached patch Patch v2.0 (obsolete) — Splinter Review
Attachment #346212 - Attachment is obsolete: true
Attachment #346616 - Flags: review?(smichaud)
Attachment #346212 - Flags: review?(smichaud)
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 :-)
Attachment #346616 - Flags: review?(smichaud) → review+
Attached patch Patch v2.0.1Splinter Review
Attachment #346616 - Attachment is obsolete: true
Attachment #346672 - Flags: superreview?(roc)
Attachment #346672 - Flags: review+
Attachment #346672 - Flags: superreview?(roc) → superreview+
Comment on attachment 346672 [details] [diff] [review]
Patch v2.0.1

We should support software keyboard on Mac.
Attachment #346672 - Flags: approval1.9.1b2?
Attachment #346672 - Flags: approval1.9.1b2? → approval1.9.1b2+
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
Flags: in-litmus?
landed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
You need to log in before you can comment on or make changes to this bug.