Closed
Bug 1395876
Opened 7 years ago
Closed 7 years ago
[TSF] If QQ input is default IME and Firefox launched with -no-remote, cannot use IME unless selecting another IME/keyboard layout
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | wontfix |
firefox57 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: inputmethod, regression)
Attachments
(1 file)
QQ input is a Chinese IME which is a TIP of TSF: http://qq.pinyin.cn/ As far as I tested on Win7 with QQ input, I see a bug similar to bug 1390097. When Firefox is launched with -no-remote, "intl.tsf.enable" is true and QQ input is the default IME, QQ input isn't "enabled" even after fixing bug 1390097. With mozregressions, I reached bug 1354020 as regression point. So, the patch for bug 1354020 still has some problems. When I switch active IME from QQ input to another keyboard layout or IME and switch it back to QQ input, it's "enabled correctly. I guess that QQ input has a bug since other TIPs don't have this bug as far as we know. I wonder, can we avoid this issue in TSFTextStore or something?
Assignee | ||
Updated•7 years ago
|
Severity: normal → major
Assignee | ||
Comment 1•7 years ago
|
||
Also reproduced on Win10.
Comment 2•7 years ago
|
||
Does it fix the issue to disable PerMonitorV2?
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #2) > Does it fix the issue to disable PerMonitorV2? Did you mean backing out the patch? Looks like there is no pref to disable it. And I can reproduce this bug on Win7. IIRC, Win7 doesn't support PerMonitorV2.
Comment 4•7 years ago
|
||
Why is this bug blocking bug 1354020, then?
Assignee | ||
Comment 5•7 years ago
|
||
Because of reaching the changeset when I use mozregressions.
Comment 6•7 years ago
|
||
That's really odd... Could you confirm that reverting bug 1354020 fixes the issue?
Comment 7•7 years ago
|
||
Even if removing PerMonitorV2 from manifest, this still occur. By bug 1390097, some changes are already reverted, so I think that this occurs by another changes...
Assignee | ||
Comment 8•7 years ago
|
||
Building backed out build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b2335988cc7e0d9234010b926bef0de98e6ee19&filter-tier=1
Comment 9•7 years ago
|
||
regression by bug 1341915?
Assignee | ||
Comment 10•7 years ago
|
||
Hmm, still not sure. I reproduced this bug with the trybuild. So, I failed to test with mozregressions. FYI: When I tested this with the trybuild first time, I couldn't reproduce this bug. I don't understand why this is not 100% reproducible... The security dialog which blocks to run untrustable exe file could affect this bug?
No longer blocks: 1354020
Comment 12•7 years ago
|
||
This is regression by bug 1341915 as long as I test on my environment. Do you have another idea without backed out?
Flags: needinfo?(aklotz)
Assignee | ||
Comment 13•7 years ago
|
||
Okay, perhaps, I got the reason. When we initialize TSF modules, we've not created "normal" window yet, there is perhaps only the message window after bug 1341915. We initialize TSF modules after creating first top level window, it works fine. I keep investigating and testing this more.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(aklotz)
Assignee | ||
Comment 14•7 years ago
|
||
FYI: However, this is really not related to bug 1390097. Even with the hack, I still see IMM-IME issue if I backed out the backout patch of bug 1390097.
Assignee | ||
Comment 15•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0e568a9f1fc366804203102c49fe2965dbed644
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
I believe that the fix is really risky. So, it must be too late for Beta.
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8905848 [details] Bug 1395876 - Initialize TSF modules after we create first normal window https://reviewboard.mozilla.org/r/177658/#review182970 ::: widget/windows/nsWindow.cpp:946 (Diff revision 1) > } > + // Some TIP and IMM may want a non-message window to initialize itself. > + // So, don't call this in the constructor of nsWindow. > + IMEHandler::Initialize(); > } > + We might have tgo add assersion when IMEHandler::Initialize isn't called.
Attachment #8905848 -
Flags: review?(m_kato) → review+
Assignee | ||
Comment 19•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905848 [details] Bug 1395876 - Initialize TSF modules after we create first normal window https://reviewboard.mozilla.org/r/177658/#review182970 > We might have tgo add assersion when IMEHandler::Initialize isn't called. What did you mean? IMEHandler::Initialize() just calls TSFTextStore::Initialize() and IMMHandler::Initialize(). So, it's stateless. Should IMMHandler has a bool flag if it's already been initialized? However, the local static variable, sFirstTopLevelWindowCreated must be enough to guarantee that it's called only once. What do you worry about and want to avoid with an assertion?
Comment 20•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905848 [details] Bug 1395876 - Initialize TSF modules after we create first normal window https://reviewboard.mozilla.org/r/177658/#review182970 > What did you mean? IMEHandler::Initialize() just calls TSFTextStore::Initialize() and IMMHandler::Initialize(). So, it's stateless. Should IMMHandler has a bool flag if it's already been initialized? However, the local static variable, sFirstTopLevelWindowCreated must be enough to guarantee that it's called only once. > > What do you worry about and want to avoid with an assertion? It is possible that IMEHandler::InitInputContext is called witout IMEHandler::Initialize. (But I think it may be no situation now , because first window will be top level window). If this situation occurs, does IME work well?
Assignee | ||
Comment 21•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905848 [details] Bug 1395876 - Initialize TSF modules after we create first normal window https://reviewboard.mozilla.org/r/177658/#review182970 > It is possible that IMEHandler::InitInputContext is called witout IMEHandler::Initialize. (But I think it may be no situation now , because first window will be top level window). If this situation occurs, does IME work well? Ah, I understand. Okay, perhaps, I should stop changing this block. Instead, IMEHandler::InitInputContext() should initialize IMEHadler::Initialize() only when the first time with local static bool variable because IMEHandler::InitInputContext() requires an HWND of an nsWindow instance. So, it's guaranteed that there is at least one normal window when it's called. How do you think?
Comment 22•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905848 [details] Bug 1395876 - Initialize TSF modules after we create first normal window https://reviewboard.mozilla.org/r/177658/#review182970 > Ah, I understand. Okay, perhaps, I should stop changing this block. Instead, IMEHandler::InitInputContext() should initialize IMEHadler::Initialize() only when the first time with local static bool variable because IMEHandler::InitInputContext() requires an HWND of an nsWindow instance. So, it's guaranteed that there is at least one normal window when it's called. How do you think? good, no problem.
Assignee | ||
Comment 23•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1480836b349b2f610bfb437f98faa08f7735b2e6
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8905848 -
Flags: review+ → review?(m_kato)
Updated•7 years ago
|
Attachment #8905848 -
Flags: review?(m_kato) → review+
Comment 25•7 years ago
|
||
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/86465f8cfaf3 Initialize TSF modules after we create first normal window r=m_kato
Comment 26•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/86465f8cfaf3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•