1208977 - [TSF][TS_E_NOLAYOUT][e10s] Candidate window position is sometimes at the top-left of screen at using MS-IME for Japanese on Win7

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Alice0775 White]

Attachment: screencast

https://hg.mozilla.org/mozilla-central/rev/6256ec9113c115141aab089c45ee69438884b680
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0 ID:20150927030300

Using Microsoft IME bundled with Japanese edition of Windows7 Home Premium

Steps To Reproduce:
1. IME on
2. Type something
3. Press Conversion Key

Actual Results:
Candidate window position is sometimes at the top-left of screen

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Now, it always flickers. annoying

Comment 2

[Alice0775 White]

Disabling e10s or disabling TSF fixes the problem.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I cannot reproduce this bug on Nightly with neither Win7, Win8.1 nor Win10. Do you really reproduce this bug with 46?

If so, could you post a log file logged by "NSPR_LOG_MODULES=nsTextStoreWidgets:5,ContentCacheWidgets:5" at following steps?
1. Start Firefox
2. Focus an editor by mouse to reproduce the bug.
3. Type something until reproducing this bug.
4. When you see this bug first time, close Firefox with clicking 'X' button of the window.

And could you confirm if "intl.tsf.hack.ms_japanese_ime.do_not_return_no_layout_error_*" are true?

Comment 4

[Alice0775 White]

Attachment: log.zip

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6) from comment #3)
> Hmm, I cannot reproduce this bug on Nightly with neither Win7, Win8.1 nor
> Win10. Do you really reproduce this bug with 46?

yes.
https://hg.mozilla.org/mozilla-central/rev/0f363ae95dc90d593394ef464aa500804c824962
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20160109030208

**You may need to change Windows7 Visual style to Aero Basic / Classic.


> 
> If so, could you post a log file logged by
> "NSPR_LOG_MODULES=nsTextStoreWidgets:5,ContentCacheWidgets:5" at following
> steps?
> 1. Start Firefox
> 2. Focus an editor by mouse to reproduce the bug.
> 3. Type something until reproducing this bug.
> 4. When you see this bug first time, close Firefox with clicking 'X' button
> of the window.
> 

Attached here.

> And could you confirm if
> "intl.tsf.hack.ms_japanese_ime.do_not_return_no_layout_error_*" are true?

Both dafault.
intl.tsf.hack.ms_japanese_ime.do_not_return_no_layout_error_at_caret;true
intl.tsf.hack.ms_japanese_ime.do_not_return_no_layout_error_at_first_char;true

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aea6e1074d4a

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you. The log file actually returns TS_E_NOLAYOUT to MS-IME for Japanese.

At checking the code, we do so only on Win8 or later since suggest window is implemented after Win8 (bug 1204523). I guess that we can fix this bug with removing the restriction.

I'll notify you of the test build becoming available.

Comment 7

[Alice0775 White]

I confirmed that the try build fixed the problem.

https://hg.mozilla.org/try/rev/3a13f8817fdfd1c20407ff454a4a810e73358aa7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20160110001900

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Alice0775 White from comment #7)
> I confirmed that the try build fixed the problem.
> 
> https://hg.mozilla.org/try/rev/3a13f8817fdfd1c20407ff454a4a810e73358aa7
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
> ID:20160110001900

Thank you very much!

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT for MS-IME for Japanese becasue it may position its candidate window at top-left of the screen in e10s mode

MS-IME for Japanese on Win7 has a bug of handling TS_E_NOLAYOUT of ITextStore::GetTextExt(), and the bug is same as suggest window positioning on Win8 and later.

So, we should enable the hack for MS-IME for Japanese even on Win7. Note that we will drop support TSF mode of WinXP, so, don't mind about it. We can just remove the restriction.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: The patch with -w

Use this for review.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43eaceeb5a48a961daa219895e1e8ee4f6975467
Bug 1208977 TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT for MS-IME for Japanese becasue it may position its candidate window at top-left of the screen in e10s mode r=emk

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8706108 [details] [diff] [review]
TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT for MS-IME for Japanese becasue it may position its candidate window at top-left of the screen in e10s mode

Approval Request Comment
[Feature/regressing bug #]: Regression of e10s mode.
[User impact if declined]: Flicking the IME candidate window os MS-IME for Japanese on Win7.
[Describe test coverage new/current, TreeHerder]: Landed onto m-c.
[Risks and why]: Low because the hack for MS-IME was restricted as only for Win8 and later, but this patch just removes the restriction for applying the hack.
[String/UUID change made/needed]: Nothing.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8706108 [details] [diff] [review]
TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT for MS-IME for Japanese becasue it may position its candidate window at top-left of the screen in e10s mode

E10s is not turned on by default on FF44. I am denying this based on the more stringent Beta uplift criteria that allows only for fixing critical regressions, sec and stability issues. To me this fix, does not meet that bar. Thanks!

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43eaceeb5a48

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8706108 [details] [diff] [review]
TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT for MS-IME for Japanese becasue it may position its candidate window at top-left of the screen in e10s mode

We will be doing more experiment with e10s on beta, taking it.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c1809400ce53