Cannot type 2 or more characters from software keyboard

RESOLVED FIXED in mozilla1.9.1b2

Status

()

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({regression})

Trunk
mozilla1.9.1b2
x86
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 346212 [details] [diff] [review]
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.
Attachment #346212 - Flags: review?(joshmoz)

Updated

10 years ago
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.
Created attachment 346616 [details] [diff] [review]
Patch v2.0
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+
Created attachment 346672 [details] [diff] [review]
Patch v2.0.1
Attachment #346616 - Attachment is obsolete: true
Attachment #346672 - Flags: superreview?(roc)
Attachment #346672 - Flags: review+
Attachment #346672 - Flags: superreview?(roc) → superreview+
Keywords: checkin-needed
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
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Depends on: 466408
You need to log in before you can comment on or make changes to this bug.