1390097 - [IME] Google Japanese Input doesn't work on Firefox started with "-no-remote" on Windows 7

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[EarlgreyTea]

Attachment: Win7 -no-remote GoogleJapandeseInput.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170809080026

Steps to reproduce:

Environment:
Windows 7 + Google Japanese Input(2.20.2750.0)

Steps to Reproduce:
1. Start firefox with "-no-remote" option and a new profile.
2. Turn on IME. ( Push [半角/全角] or [Alt] + ['] )
3. Input some Japanese. (ex 'あいうえお')


Actual results:

Actual Results:
IME does not turn on, typed characters are entered directly. (ex. 'aiueo')


Expected results:

Expected Results:
Can input and convert Japanese.

Fx 55.0.1 (64bit) : bad
Fx 56.0b2 (64bit) : bad
Fx 57.0a1 (2017-08-13) (64bit) : good

It doesn't occur when using MS-IME or on Windows 10.

Comment 1

[Makoto Kato [:m_kato]]

(In reply to EarlgreyTea from comment #0)
> Fx 57.0a1 (2017-08-13) (64bit) : good

As long as I test this, this still occurs even if using Nightly 2017-08-16.

Comment 2

[EarlgreyTea]

(In reply to Makoto Kato [:m_kato] from comment #1)
> As long as I test this, this still occurs even if using Nightly 2017-08-16.

I tested this again, but again I did not reproduce with Nightly 2017-08-13 and 2017-08-18.

Environment:
UA: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
OS: Microsoft Windows 7 Professional Version 6.1.7601 Service Pack 1 Build 7601
IME: Google Japanese Input Version 2.20.2750.0

Incidentally, because I used an Internet cafe PC, I have not reboot after installing Google Japanese Input.

Comment 3

[EarlgreyTea]

Attachment: Nightly(2017-08-18) can input japanese.jpg

Comment 4

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

Google Japanese Input is installed as IMM-IME on Win7, but not so, i.e., as TIP of TSF on Win8 and later. Even if I reproduce this, I can enable IME forcibly with selecting TIP as active IME (even after switching IME back to Google Japanese Input, it's available).

As far as I tested, when IMM-IME is default IME and active IME is NOT shared between processes, I reproduced this bug. However I couldn't reach reasonable regression point with mozregression...

I don't know what is different between normal mode and no-remote mode.

Comment 5

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

FYI: If Google Japanese Input hasn't been updated after upgrading from Win7 to Win8 or later, it's still an IMM-IME. You can make it TIP after reinstalling it.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Given comment 4, let's say it is confirmed. Someone reported on Twitter to me that this happens for Chinese input method as well, although I'm not sure what IM is installed in that case.

Comment 7

[Makoto Kato [:m_kato]]

When DDE is started, it can activate IME.  NO_REMOTE doesn't start DDE.  This is strange issue.

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Note sure what DDE is... but sounds like this code may be related? https://searchfox.org/mozilla-central/rev/b258e6864ee3e809d40982bc5d0d5aff66a20780/toolkit/xre/nsNativeAppSupportWin.cpp#623

Comment 9

[Makoto Kato [:m_kato]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #8)
> Note sure what DDE is... but sounds like this code may be related?
> https://searchfox.org/mozilla-central/rev/
> b258e6864ee3e809d40982bc5d0d5aff66a20780/toolkit/xre/nsNativeAppSupportWin.
> cpp#623

This doesn't occur when nsNativeAppSupportWin::StartDDE is called.   If NO_REMOTE, this isn't called.

Comment 10

[Makoto Kato [:m_kato]]

more strange...  This seems to be regression by PerMonitorV2 support.

Comment 11

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

I reached same changeset when I tested on my VM, but it doesn't make sense to me too.

Comment 12

[Masatoshi Kimura [:emk]]

(In reply to Makoto Kato [:m_kato] from comment #10)
> more strange...  This seems to be regression by PerMonitorV2 support.

That's indeed strange. How can PerMonitorV2 support affect Windows 7 where (even v1) Per-monitor DPI is not supported at all?

Comment 13

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

Is it possible to hit a bug of Windows of manifest handling?

Comment 14

[Makoto Kato [:m_kato]]

moz-regression points out that bug 1354020 is regression bug.  But even if I remove PerMonitor DPI setting from manifest simply, it is still reproduced.  So I am still investigating root cause...

Comment 15

[Makoto Kato [:m_kato]]

Another change of bug 1354020 causes this regression.  Maybe, IME? will check whether default procedure is DefWindowProcW or not.  Before landing this, it was NonClientScallingDefWindowProcW as default.

Comment 16

[Makoto Kato [:m_kato]]

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

Comment 17

[Makoto Kato [:m_kato]]

Attachment: Bug 1390097 - Revert a part of bug 1354020 changes.

Bug 1354020 causes that Google Japanese IME doesn't work with --no-remote.  Although I think that this issue is OS or IME bug, when default window proceduce by RegisterClass is DefaultWindowProcW, Google Japanese IME doesn't work.

I am not sure why this issue occurs when lpfnWndProc is DefWidnowProcW and DDE isn't started.  But for workaround, we should revert a part of bug 1354020 changes.

Review commit: https://reviewboard.mozilla.org/r/171468/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171468/

Comment 18

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

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

https://reviewboard.mozilla.org/r/171468/#review176636

::: commit-message-7c50f:3
(Diff revision 1)
> +Bug 1390097 - Revert a part of bug 1354020 changes. r?masayuki
> +
> +Bug 1354020 causes that Google Japanese IME doesn't work with --no-remote.  Although I think that this issue is OS or IME bug, when default window proceduce by RegisterClass is DefaultWindowProcW, Google Japanese IME doesn't work.

I think that this message sounds like Google Japanese Input does something wrong on any version of Windows. Like the similar bug with Chinese IME, I guess that this occurs only with IMM-IME on Win7 (I'm not sure about Win8.1, I confirmed that this won't occur on Win10 since Win10 cannot make IMM-IME default IME).

So, I'd like to suggest:

s/Google Japanese IME/IMM-IME on Win7

::: commit-message-7c50f:5
(Diff revision 1)
> +Bug 1390097 - Revert a part of bug 1354020 changes. r?masayuki
> +
> +Bug 1354020 causes that Google Japanese IME doesn't work with --no-remote.  Although I think that this issue is OS or IME bug, when default window proceduce by RegisterClass is DefaultWindowProcW, Google Japanese IME doesn't work.
> +
> +I am not sure why this issue occurs when lpfnWndProc is DefWidnowProcW and DDE isn't started.  But for workaround, we should revert a part of bug 1354020 changes.

Will this break the Per-monitor DPI v2 support from Gecko? If so, it's too bad.

As your commit message, I wonder, does creating dummy window proc which just wraps DefWindowProcW fix this bug?

Comment 19

[Makoto Kato [:m_kato]]

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

https://reviewboard.mozilla.org/r/171468/#review176662

::: commit-message-7c50f:3
(Diff revision 1)
> +Bug 1390097 - Revert a part of bug 1354020 changes. r?masayuki
> +
> +Bug 1354020 causes that Google Japanese IME doesn't work with --no-remote.  Although I think that this issue is OS or IME bug, when default window proceduce by RegisterClass is DefaultWindowProcW, Google Japanese IME doesn't work.

OK.

::: commit-message-7c50f:5
(Diff revision 1)
> +Bug 1390097 - Revert a part of bug 1354020 changes. r?masayuki
> +
> +Bug 1354020 causes that Google Japanese IME doesn't work with --no-remote.  Although I think that this issue is OS or IME bug, when default window proceduce by RegisterClass is DefaultWindowProcW, Google Japanese IME doesn't work.
> +
> +I am not sure why this issue occurs when lpfnWndProc is DefWidnowProcW and DDE isn't started.  But for workaround, we should revert a part of bug 1354020 changes.

No, this keeps PerMonitorV2 support.

>  I wonder, does creating dummy window proc which just wraps DefWindowProcW fix this bug?

Yes, maybe, OS(IMM?) checks whether current WindowProc is DefWindowProcW address.  But I don't know why this cleanup change causes this issue.

Comment 20

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

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

https://reviewboard.mozilla.org/r/171468/#review176662

> No, this keeps PerMonitorV2 support.
> 
> >  I wonder, does creating dummy window proc which just wraps DefWindowProcW fix this bug?
> 
> Yes, maybe, OS(IMM?) checks whether current WindowProc is DefWindowProcW address.  But I don't know why this cleanup change causes this issue.

I wonder, it might be wrong IMMHandler to call ::DefWindowProc() directly:
https://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/widget/windows/IMMHandler.cpp#1004,1046
https://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/widget/windows/IMMHandler.cpp#1186,1213

According to the log of Nightly, even if IME should be enabled, "nsIMM32HandlerWidgets OnIMESetContext, hWnd=XXXXXXXX, Dective" is logged after "nsIMM32HandlerWidgets OnIMESetContext, hWnd=XXXXXXXX, Active". However, with your trybuild, I don't see the redundant OnIMESetContext calls. I guess that directly calling ::DefWindowProc() causes bypassing something of CUAS due to not default wndproc of the window class.

Comment 21

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

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

https://reviewboard.mozilla.org/r/171468/#review176736

Okay, perhaps, partial backout must be the safest way for uplift. I think that we should take this.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Once this lands on m-c, please verify the fix and then let's uplift to at least beta 56.

Comment 23

[Makoto Kato [:m_kato]]

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171468/diff/1-2/

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad553bca3aac
Revert a part of bug 1354020 changes. r=masayuki

Comment 25

[Makoto Kato [:m_kato]]

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1354020

[User impact if declined]:
When launching Firefox on Windows 7 with -no-remote option, some 3rd party IME doesn't work.  It means that user cannot input Japanese, Korean and Chinese.

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
I test on my local environment.

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes.

1. Setup Windows 7 x64
2. Install Google Japanese Input from https://www.google.co.jp/ime/
3. Install Firefox x64
4. Launch Firefox
5. Focus to search box
6. Turn on IME (In US keyboard, [Alt] + [~])
7. Type [a] key

Expected Result
あ is inputted.

[List of other uplifts needed for the feature/fix]:
No

[Is the change risky?]:
Low.

[Why is the change risky/not risky?]:
Use wrapper window procedure to call DefWindowProc instead of call directly.

[String changes made/needed]:
No

Comment 26

[Makoto Kato [:m_kato]]

I forget that changing to Google Japanese Input in steps.  So correct step is the following.

1. Setup Windows 7 x64
2. Install Google Japanese Input from https://www.google.co.jp/ime/
3. Install Firefox x64
4. Launch Firefox
5. Change IME to Google Japanese Input from task tray.
6. Focus to search box
7. Turn on IME (In US keyboard, [Alt] + [~])
8. Type [a] key

Comment 27

[Makoto Kato [:m_kato]]

Also, if possible, QA team need verify no regression of bug 1354020 issue.  (HiDPI w/ multiple monitor support works well even if this fix is applied)

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ad553bca3aac

Comment 29

[Ciprian Georgiu, Desktop QA]

I've reproduced this issue on an affected Nightly build (2017-08-14) using STR from comment 26.

I've managed to verify this on an autoland build (https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ad553bca3aac60b2d75d292e4a129892e4840bf3&selectedJob=125379234), running Windows 7 x64, and I can confirm that Google Japanese Input works as expected.

(In reply to Makoto Kato [:m_kato] from comment #27)
> Also, if possible, QA team need verify no regression of bug 1354020 issue. 
> (HiDPI w/ multiple monitor support works well even if this fix is applied)

I will follow-up with the testing results for this scenario as well, as soon as testing is completed. Thanks!

Comment 30

[Julien Cristau [:jcristau]]

As an update on 55 status: I'm keeping this out of 55.0.3 as the fix isn't specific to IME code and so I'd like it to get some baking in pre-release channels for fear of breaking something else, especially as per comment 29 the manual regression testing isn't done yet, and -no-remote seems a bit of a corner case.  If we end up needing a 55.0.4 we can reconsider then.

Comment 31

[Makoto Kato [:m_kato]]

Remove bug 1394112 since that issue isn't related.

Comment 32

[Gerry Chang [:gchang]]

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

Fix a regression and was verified. Beta56+.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f2c71d60be69

Comment 34

[Ciprian Georgiu, Desktop QA]

I've finished the manual regression testing, running Windows 10 x64 (OS version 1607), Windows 7 x86 and Windows 8.1 x64 on latest Nightly 57.0a1 and Beta 56.0b7. I did encounter a few possible issues while testing this (see [1], [2], [3]), but it doesn't seem to be recent regressions.

Here are some scenarios that I covered through exploratory testing:
- dragging Firefox from one screen to another having different display resolutions
- web compact on popular websites (e.g.: youtube.com. instagram.com, facebook.com, imgur.com, etc.)
- inspect the Firefox buttons, toolbars, dialog boxes
- checking Firefox with Compact themes applied

Testing was done on the following configuration:
- Low resolution monitor (WSXGA Samsung 2233)
- High resolution monitor (Dell UHD-1 P2415Qb)

Possible issues:
[1] Artifacts left behind when the Download manager's width is resized.
- I was able to repro this on a a HiDPI monitor and with different scaling levels e.g. 125%, 200%. It appears that this issue is still present on Windows 10 if I Sign out and Sign in back. On Win 8.1 and Win 7 I'm not seeing this artifacts, please see this screencast: http://imgur.com/a/2OEDW.

[2] White border displayed if Firefox is maximized
- as can be observed in this screenshot: http://imgur.com/gallery/w5hM3, the border is displayed in the left side and on the bottom, but it has only a few pixels.
- if the screenshot is zoom in, some bluish artifacts are displayed in the right side of the Menu button.
- specific to Win 10

[3] Dialog boxes are not scaled if Firefox is dragged from a displayed with a lower resolution to another one with a higher resolution
- specific to Win 10, the lower resolution display had 125% level scaled and the higher one 200%
- please see this screenshot: http://imgur.com/a/CPoAh

Makoto, please let me know what you think about this issues and if I should file them!

Since the STR from comment 26 are not reproducible anymore on 56.0b7 (20170828185355) and latest Nightly 57.0a1 (2017-08-30) running Windows 7 x64, I'm marking this verified fixed.

Comment 35

[Makoto Kato [:m_kato]]

Ciprian, thanks for testing!

(In reply to Ciprian Georgiu, QA [:ciprian_georgiu] from comment #34)
> I've finished the manual regression testing, running Windows 10 x64 (OS
> version 1607), Windows 7 x86 and Windows 8.1 x64 on latest Nightly 57.0a1
> and Beta 56.0b7. I did encounter a few possible issues while testing this
> (see [1], [2], [3]), but it doesn't seem to be recent regressions.
> 
> Here are some scenarios that I covered through exploratory testing:
> - dragging Firefox from one screen to another having different display
> resolutions
> - web compact on popular websites (e.g.: youtube.com. instagram.com,
> facebook.com, imgur.com, etc.)
> - inspect the Firefox buttons, toolbars, dialog boxes
> - checking Firefox with Compact themes applied
> 
> Testing was done on the following configuration:
> - Low resolution monitor (WSXGA Samsung 2233)
> - High resolution monitor (Dell UHD-1 P2415Qb)
> 
> Possible issues:
> [1] Artifacts left behind when the Download manager's width is resized.
> - I was able to repro this on a a HiDPI monitor and with different scaling
> levels e.g. 125%, 200%. It appears that this issue is still present on
> Windows 10 if I Sign out and Sign in back. On Win 8.1 and Win 7 I'm not
> seeing this artifacts, please see this screencast: http://imgur.com/a/2OEDW.

This is new issue even if I can reproduce before landing this.  So could you file a new bug?

> [2] White border displayed if Firefox is maximized
> - as can be observed in this screenshot: http://imgur.com/gallery/w5hM3, the
> border is displayed in the left side and on the bottom, but it has only a
> few pixels.
> - if the screenshot is zoom in, some bluish artifacts are displayed in the
> right side of the Menu button.
> - specific to Win 10

I cannot reproduce this, but it seems new issue, please file a new bug

 
> [3] Dialog boxes are not scaled if Firefox is dragged from a displayed with
> a lower resolution to another one with a higher resolution
> - specific to Win 10, the lower resolution display had 125% level scaled and
> the higher one 200%
> - please see this screenshot: http://imgur.com/a/CPoAh

Also, this is new issue even if I can still reproduce on 55.  So could you file a new bug?

Comment 37

[Ciprian Georgiu, Desktop QA]

Filed bug 1396816 [1], bug 1396840 [2] and bug 1396807 [3] per comment 35.

Comment 38

[Julien Cristau [:jcristau]]

Comment on attachment 8900104 [details]
Bug 1390097 - Revert a part of bug 1354020 changes.

We're two weeks away from 56 release, and there are no plans for 55.0.4 at this time.