Closed Bug 838001 Opened 7 years ago Closed 7 years ago

Input Method can't be opened to type to input fields in a panel on Firefox 19+

Categories

(Core :: Internationalization, defect, major)

x86_64
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla21
Tracking Status
firefox18 --- unaffected
firefox19 + verified
firefox20 + verified
firefox21 + verified
firefox-esr17 --- unaffected

People

(Reporter: pzhang, Assigned: masayuki)

References

Details

(Keywords: inputmethod, regression)

Attachments

(2 files)

I'm able to type into the panel's textbox when I click the widget in Firefox 21 Nightly builds, for what it's worth. Can you explain the problem a bit more?
Flags: needinfo?(pzhang)
(In reply to Wes Kocher (:KWierso) from comment #1)
> I'm able to type into the panel's textbox when I click the widget in Firefox
> 21 Nightly builds, for what it's worth. Can you explain the problem a bit
> more?

Did you try any CJK IME to type Chinese/Japanese/Korean characters?
Flags: needinfo?(pzhang)
What's the "panel"? If it's XUL's <panel>, you must not be able to use IME on Firefox's bookmark panel too. How about that? I can use IME on the bookmark panel of Nightly.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3)
> What's the "panel"? If it's XUL's <panel>, you must not be able to use IME
> on Firefox's bookmark panel too. How about that? I can use IME on the
> bookmark panel of Nightly.

I don't know if the "panel" is implemented by the XUL's <panel>, but it works on FF 18-.

The Add-on SDK provides the panel model[1] to show a whatever page in a panel, for example, if I create a panel to show Google page, then I can't search any CJK keywords anymore, so obviously, this is a regression.

This bug will affect all the users in China, Japan, Korea and other foreign input method users.

[1] https://addons.mozilla.org/en-US/developers/docs/sdk/latest/modules/sdk/panel.html
Okay, this is a regression of bug 802896, bug 705057 and bug 805306.

I made nsPresContext::GetNearestWidget() for TextComposition class. I stole the code from nsDOMWindowUtils::GetWidget(). And the changes for bug 802896 and bug 805306, I removed nsIMEStateManager::GetWidget() and the made the callers (IME enabled state management code) use nsPresContext::GetNearestWidget().

By this change, when the panel's <input> gets focus, nsIMEStateManager changes wrong widget's IME state. nsIMEStateManager::GetWidget() had always changed the root widget of the nsPresContext. In most cases, the result of nsDOMWindowUtils::GetWidget() and nsIMEStateManager::GetWidget() are same except this case.

I still don't understand perfectly what happens in this case. I guess that the panel makes new nsPresContext and a child widget. And current code changes the IME state of the child widget. However, the root widget has native focus. Then, the root widget's IME state is referred in this case.

So, I think that I should make nsPresContext::GetRootWidget() which is same as the removed nsIMEStateManager::GetWidget(). And the callers which had called nsIMEStateManager::GetWidget() should use the new method.
Assignee: nobody → masayuki
Blocks: 802896, 705057, 805306
Severity: normal → major
Status: NEW → ASSIGNED
Component: General → Internationalization
Product: Add-on SDK → Core
Version: unspecified → Trunk
Comment on attachment 710190 [details] [diff] [review]
Patch

In fact, this is a backout patch. nsIMEStateManager::GetWidget() is replaced with nsPresContext::GetRootWidget() if we have not landed the preceding patches.

FYI: There is already another user of GetNearestWidget(), so, we need to keep it.

And I guess that nsDOMWindowUtils::GetWidget() might be wrong at least for keyboard and IME API. I feel that it's odd nsIMEStateManager::GetWidget() and nsDOMWindowUtils::GetWidget() may return different widget.
Attachment #710190 - Flags: review?(bugs)
Attachment #710190 - Flags: review?(bugs) → review+
Please nominate for uplift asap, with user impact/risk. If we're going to take a fix here for FF19, it needs to be for tomorrow's beta and approved/uplifted tonight (PT).
Comment on attachment 710190 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 802896 whose nsPresContext::GetNearestWidget() is introduced in bug 705057. And bug 805306 must cause similar symptom.

User impact if declined:
CJK users cannot use IME on the "panel" which is created by the Add-on SDK. This is a serious a11y regression. I'm not sure that such UI is used by how many add-ons though.

Testing completed (on m-c, etc.):
Just landed on m-i.

Risk to taking this patch (and alternatives if risky):
I think the risk is minimum. When I make nsPresContext::GetNearestWidget() for implementing TextComposition class, I stole the logic from nsDOMWindowUtils::GetWidget(). However, it's different logic from nsIMEStateManager::GetWidget(). But I hadn't found the difference of the result actually. Therefore, nsIMEStateManager::GetWidget() callers are replaced with nsPresContext::GetNearestWidget() in bug 802896 and bug 805306. Now, we find the different case of these methods' result. This patch makes nsPresContext::GetRootWidget() which is same as nsIMEStateManager::GetWidget(). And all IME related code which needs to access widget for accessing native IME use the new method. In other words, this backouts the change of nsIMEStateManager::GetWidget(). And the method is moved to nsPresContext for being shared with nsIMEStateManager and TextComposition.

String or UUID changes made by this patch:
Nothing.
Attachment #710190 - Flags: approval-mozilla-aurora?
Comment on attachment 710272 [details] [diff] [review]
Patch for Beta

[Approval Request Comment]

See the previous comment. The difference between the patch and the patch for m-c and Aurora is, whether nsPresContext::GetRootWidget() uses nsViewManager or nsIViewManager.

nsIViewManager has been removed on Mozilla 20. Therefore, for Beta, we need this patch.
Attachment #710272 - Flags: approval-mozilla-beta?
Comment on attachment 710190 [details] [diff] [review]
Patch

Approving this backout for Aurora and Beta. Can Mozilla Japan help test beta 5, once available, to ensure there haven't been any major CJK IME regressions from this? Thanks!
Attachment #710190 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #710272 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/mozilla-central/rev/38218fd7cdaf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Pin Zhang, can you please confirm this is now fixed for you? It should be fixed in the following builds:

Firefox Beta 19.0b5 (currently building)
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/19.0b5-candidates/build1/

Firefox Aurora 20.0a2 (as of 2013-02-07)
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora

Firefox Nightly 21.0a1 (as of 2013-02-07)
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central

Thank you
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #16)
> Pin Zhang, can you please confirm this is now fixed for you? It should be
> fixed in the following builds:
> 
> Firefox Beta 19.0b5 (currently building)
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/19.0b5-candidates/
> build1/
> 
> Firefox Aurora 20.0a2 (as of 2013-02-07)
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora
> 
> Firefox Nightly 21.0a1 (as of 2013-02-07)
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central
> 
> Thank you

I tested the 19.0b5, it perfectly works both on Linux and Windows, that's cool~

The latest Aurora and Nightly are still the builds of 2013-02-06 right now, I will verify them later today.

Thanks~
Thanks for your help, Pin.
(In reply to Pin Zhang [:pzhang] from comment #17)
> (In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #16)
> > Pin Zhang, can you please confirm this is now fixed for you? It should be
> > fixed in the following builds:
> > 
> > Firefox Beta 19.0b5 (currently building)
> > ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/19.0b5-candidates/
> > build1/
> > 
> > Firefox Aurora 20.0a2 (as of 2013-02-07)
> > ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora
> > 
> > Firefox Nightly 21.0a1 (as of 2013-02-07)
> > ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central
> > 
> > Thank you
> 
> I tested the 19.0b5, it perfectly works both on Linux and Windows, that's
> cool~
> 
> The latest Aurora and Nightly are still the builds of 2013-02-06 right now,
> I will verify them later today.
> 
> Thanks~

The 2013-02-08 builds of Aurora and Nightly are verified on both Linux and Windows, the problem is fixed.
(In reply to Pin Zhang [:pzhang] from comment #19)
> The 2013-02-08 builds of Aurora and Nightly are verified on both Linux and
> Windows, the problem is fixed.

Thank you.
Status: RESOLVED → VERIFIED
Blocks: 829946
You need to log in before you can comment on or make changes to this bug.