New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 657623 link

Starred by 9 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Feature

Blocking:
issue 557798
issue 794409



Sign in to add a comment

Using TSF instead of IMM32 in the IMF on Windows

Project Member Reported by penny...@chromium.org, Oct 19 2016

Issue description


1) Change the IME internals to not use legacy hooking techniques.

2) Add at least one automated true/false test to check that IMEs are "working".  

Ref: https://bugs.chromium.org/p/chromium/issues/detail?id=634192#c45

 
 Issue 657625  has been merged into this issue.
Summary: Using TSF instead of IMM32 in the IMF on Windows (was: Update Chromium IME support implementation (no more legacy hooking))

Comment 3 by yosin@chromium.org, Apr 20 2017

Cc: yukawa@chromium.org
AFAIK, we implemented TSF support for Chrome to support Win8 Metro mode, 
then remove it when Chrome stopped to support Win8 Metro mode.

So, we can get the code from our code base. yukawa@ might know details.

Comment 4 by yukawa@chromium.org, Aug 28 2017

Cc: wfh@chromium.org
wfh@: Do you know anyone who can work on this?  Edge and Firefox have basically switched to new IME APIs called Text Services Framework on Windows while Chromium (hence its downstream projects such as Electron) is still using legacy IME APIs, which seems to be a major blocker to ebable ProcessExtensionPointDisablePolicy at bug 557798.

Can Chrome team allocate more engineering resource on IME support on Windows?

Re comment 3:
> So, we can get the code from our code base. yukawa@ might know details.

That code had lots of known issues.  From what I saw from the Mozilla's effort to switch to TSF in Firefox, I'd expect at least 1 SWE year to complete the migration.

Comment 5 by wfh@chromium.org, Aug 28 2017

Owner: rpop@chromium.org
Status: Assigned (was: Available)
rpop - do you know about resourcing for this..? see #c4

Comment 6 by yosin@chromium.org, Sep 20 2017

Cc: yosin@chromium.org
Components: Blink>Editing>IME

Comment 7 by rpop@chromium.org, Oct 3 2017

Cc: -jsc...@chromium.org rpop@chromium.org
Owner: jsc...@chromium.org
I'm not sure. Jschuh is this something windows team should do or that someone in Blink with Windows familiarity might be able to do? Or if this is urgent for Electron, is there an external contributor who might step up?
Historically the Tokyo office covered IME, because they have the relevant expertise and a solid appreciation for how it should work. But staffing has shifted and I'm not sure if there's anyone available to work on it now.

Comment 9 by kochi@chromium.org, Oct 4 2017

Yes, people in Tokyo worked when Windows8 Metro mode was introduced
for TSF stuff, but they moved to other teams/offices so they are no
longer working on input stuff.

That said, the code for TSF was removed when Metro mode code was all removed
from the code base, we should be able to revive the code and reuse most of
the part, which is significantly better than starting from scratch.
Cc: jsc...@chromium.org
Owner: dtapu...@chromium.org

Comment 11 by yosin@chromium.org, Feb 15 2018

Status: Started (was: Assigned)
In review: https://chromium-review.googlesource.com/c/chromium/src/+/916534
Blocking: 794409
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1c1d3de40d7b19358b219bab7cec0042c3ed337

commit e1c1d3de40d7b19358b219bab7cec0042c3ed337
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Thu Mar 08 22:06:15 2018

Resurrect Text Services Framework support on Windows.

The bulk of this change is to revert commit
0b15dad9d560c7a85c854aeae152f6a66757b5e1. It isn't a direct revert due
to a number of things.
  - The ScopedComPtr interface that was widely used in the old patch no
    longer exists.
  - A number of interfaces change slightly (ImeTextSpans)
  - unsigned/signed compiler warnings

This code was abandoned a few years ago and removed from the repository
because it only worked in the Windows 8 Metro Mode of Chrome. Once that
mode was removed the supporting IME code was removed as well. The code
in its current form is functional but has issues which will be addressed
in follow up patches. It is desirable to move to TSF since there are
additional APIs that work via this API. Like text writing.

Functionality common to both input methods are placed in
InputMethodWinBase. InputMethodWin will be renamed to InputMethodWinImm32
in a followup patch.

This feature is currently disabled but can be enabled via
--enable-features=TSFImeSupport. No functionality changes are expected
with the Imm32 implementation.

Inputing text using Pinyin seems to work; as well as disabling DLL
injection (crbug.com/557798) allows the IME to work correctly with TSF.

There is some broken support with this change in that keydown/up messages,
arrow keys in the omnibox are not delivered. I intend to fix these in
followup changes being that this change is intended to make a previously
reverted change available under a command line flag.

BUG=657623

Change-Id: I89eb1a7ba6b9f95525df6fa83be5d923a03323b2
Reviewed-on: https://chromium-review.googlesource.com/916534
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541895}
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/BUILD.gn
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/BUILD.gn
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_base.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_base.h
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_factory.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_initializer.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win_base.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win_base.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win_tsf.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/input_method_win_tsf.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/mock_tsf_bridge.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/mock_tsf_bridge.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_bridge.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_bridge.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_event_router.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_event_router.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_text_store.cc
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_text_store.h
[add] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ime/win/tsf_text_store_unittest.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ui_base_features.cc
[modify] https://crrev.com/e1c1d3de40d7b19358b219bab7cec0042c3ed337/ui/base/ui_base_features.h

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/addfa4e62cd0bc3f83a88d34e66e08ceb7a08ea8

commit addfa4e62cd0bc3f83a88d34e66e08ceb7a08ea8
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Mar 09 14:55:29 2018

[jumbo] Windows build fix related to INITGUID

Generating GUID objects in a binary requires careful juggling
of includes and the define INITGUID (or the header <Initguid.h>).

Jumbo builds are not that careful so the simplest/best solution
is to remove the file doing INITGUID from jumbo compilation.
This manifested itself in jumbo fyi builders building
catalog_viewer_service.exe.

TBR=dtapuska@chromium.org

Bug: 657623
Change-Id: I1457727a0b3a68f82726a2e73c1ae3dedb06f12d
Reviewed-on: https://chromium-review.googlesource.com/955690
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#542108}
[modify] https://crrev.com/addfa4e62cd0bc3f83a88d34e66e08ceb7a08ea8/ui/base/ime/BUILD.gn

Owner: lanwei@chromium.org
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a2c7b5b9e8890b89397692007ff26e42784c3acf

commit a2c7b5b9e8890b89397692007ff26e42784c3acf
Author: lanwei <lanwei@chromium.org>
Date: Thu Aug 02 15:21:57 2018

Rename InputMethodWin to InputMethodWinImm32

Since we start to switch to TSF from IMM32 in IME on Windows, we want
to rename InputMethodWin class to InputMethodWinImm32 to distinguish
these two input methods.

Bug: 657623
Change-Id: I70fc2f6ee319fd050e06e5937c10dbf4996681f7
Reviewed-on: https://chromium-review.googlesource.com/1159351
Reviewed-by: Shu Chen <shuchen@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Lan Wei <lanwei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580184}
[modify] https://crrev.com/a2c7b5b9e8890b89397692007ff26e42784c3acf/ui/base/ime/BUILD.gn
[modify] https://crrev.com/a2c7b5b9e8890b89397692007ff26e42784c3acf/ui/base/ime/input_method_factory.cc
[rename] https://crrev.com/a2c7b5b9e8890b89397692007ff26e42784c3acf/ui/base/ime/input_method_win_imm32.cc
[rename] https://crrev.com/a2c7b5b9e8890b89397692007ff26e42784c3acf/ui/base/ime/input_method_win_imm32.h

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/915ab7e9eb588029727173985352ff47d60bdb26

commit 915ab7e9eb588029727173985352ff47d60bdb26
Author: lanwei <lanwei@chromium.org>
Date: Wed Aug 15 22:44:37 2018

Move DispatchKeyEvent to InputMethodWinBase

The DispatchKeyEvent in InputMethodWinBaseIMM32 does not have any code
special to IMM32, so I move it to InputMethodWinBase, which can be
shared by both InputMethodWinBaseIMM32 and InputMethodWinBaseTSF.

This CL does not have any new behavior change. This DispatchKeyEvent
works when TSF is enabled, but I will try to use TSF API to replace
PeekMessage API in the following CLs.


Bug: 657623
Change-Id: I0b03f7b393660676ff26df1a9bdbfbbd43dc6a45
Reviewed-on: https://chromium-review.googlesource.com/1173336
Commit-Queue: Lan Wei <lanwei@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Reviewed-by: Yohei Yukawa <yukawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583423}
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_base.cc
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_base.h
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_imm32.cc
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_imm32.h
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_tsf.cc
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/input_method_win_tsf.h
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/win/imm32_manager.cc
[modify] https://crrev.com/915ab7e9eb588029727173985352ff47d60bdb26/ui/base/ime/win/imm32_manager.h

Hi,

I was wondering if this bug was still being worked on? We'd really like to fix 557798 but it's blocked on this one.

Thanks!

Sign in to add a comment