|
|
Created:
7 years, 5 months ago by yoichio Modified:
7 years, 4 months ago CC:
chromium-reviews, nona+watch_chromium.org, James Su, penghuang+watch_chromium.org, yusukes+watch_chromium.org, kochi Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
Descriptionadd IMM32Manager::SetTextInputMode() function.
This function changes IME conversion status corresponding to |input_mode| such as "latin", "katakana".
BUG=244688
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214577
Patch Set 1 : update #
Total comments: 6
Patch Set 2 : split function #
Total comments: 27
Patch Set 3 : update #
Total comments: 6
Patch Set 4 : nit pick #Patch Set 5 : add comment #Patch Set 6 : treat latin* as same as default #
Total comments: 8
Patch Set 7 : nit picks #
Messages
Total messages: 20 (0 generated)
Could you review?
https://codereview.chromium.org/19690005/diff/16001/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager.cc (right): https://codereview.chromium.org/19690005/diff/16001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.cc:533: | IME_CMODE_LANGUAGE Why do we need IME_CMODE_LANGUAGE? AFAIK, IME_CMODE_LANGUAGE is equivalent to (IME_CMODE_JAPANESE | IME_CMODE_KATAKANA). Could you double check it? https://codereview.chromium.org/19690005/diff/16001/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://codereview.chromium.org/19690005/diff/16001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager_unittest.cc:59: TEST_F(IMM32ManagerTest, SetTextInputModeTest) { How about splitting SetTextInputMode into core mapping logic and Win32 dependent code and testing only the former one ? How about using TESF_P or something? https://code.google.com/p/googletest/wiki/AdvancedGuide#How_to_Write_Value-Pa... https://codereview.chromium.org/19690005/diff/16001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager_unittest.cc:65: const DWORD expected_set_conversion_mode; Instead of testing only modified bits, we should have test cases where "the initial conversion mode" and "the last (expected) conversion mode" are listed.
Get out ConvertInputModetoImmFlags from SetTextInputMode. https://chromiumcodereview.appspot.com/19690005/diff/16001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/16001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.cc:533: | IME_CMODE_LANGUAGE On 2013/07/25 06:53:29, Yohei Yukawa wrote: > Why do we need IME_CMODE_LANGUAGE? > AFAIK, IME_CMODE_LANGUAGE is equivalent to (IME_CMODE_JAPANESE | > IME_CMODE_KATAKANA). Could you double check it? Delete IME_CMODE_LANGUAGE. https://chromiumcodereview.appspot.com/19690005/diff/16001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/16001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:59: TEST_F(IMM32ManagerTest, SetTextInputModeTest) { On 2013/07/25 06:53:29, Yohei Yukawa wrote: > How about splitting SetTextInputMode into core mapping logic and Win32 dependent > code and testing only the former one ? > > How about using TESF_P or something? > https://code.google.com/p/googletest/wiki/AdvancedGuide#How_to_Write_Value-Pa... Done. https://chromiumcodereview.appspot.com/19690005/diff/16001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:65: const DWORD expected_set_conversion_mode; On 2013/07/25 06:53:29, Yohei Yukawa wrote: > Instead of testing only modified bits, we should have test cases where > "the initial conversion mode" and "the last (expected) conversion mode" are > listed. Done.
Added nona@ for hearing comment at early state. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.h (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:243: // Get parameters for ::ImmSetOpenStatus and ::ImmSetConversionStatus from nit: Gets https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, nit: ConvertInputModeToImmFlags https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, This can be static method. And how about simply converting |initial_conversion_mode| into |new_conversion_mode| instead of returning the bit masks? void ConvertInputModeToImmFlags(ui::TextInputMode input_mode, DWORD initial_conversion_mode, DWORD* new_conversion_mode, BOOL* open); https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:249: // Set conversion status corresponding to |input_mode|. nit: Sets https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:9: namespace { How about putting all the stuff (including unit test itself) into anonymous namespace? e.g. namespace ui { namespace { struct InputModeTestCase {...} class IMM32ManagerTest... const InputModeTestCase kInputModeTestCases[] = { ... }; TEST_P(...) INSTANTIATE_TEST_CASE_P(...) } // namespace } // namespace ui https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:27: {TEXT_INPUT_MODE_DEFAULT, 0,FALSE, 0}, nit: a white space is missing after 0, https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:27: {TEXT_INPUT_MODE_DEFAULT, 0,FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:28: {TEXT_INPUT_MODE_VERBATIM, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:49: {TEXT_INPUT_MODE_KATAKANA, We might want to have some additional test cases to make sure other bit flags are preserved. For instance, when Hiragana-mode is enabled with Romaji-style, the test case should be as follows. {TEXT_INPUT_MODE_KANA, IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN, TRUE, IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN}, (We should preserve IME_CMODE_ROMAN in this case.) https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:53: {TEXT_INPUT_MODE_NUMERIC, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:54: {TEXT_INPUT_MODE_TEL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:55: {TEXT_INPUT_MODE_EMAIL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:56: {TEXT_INPUT_MODE_URL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:90: ::testing::ValuesIn( I guess we can simply write ::testing::ValuesIn(kInputModeTestCases)
Added nona@ for hearing comment at early state. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.h (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:243: // Get parameters for ::ImmSetOpenStatus and ::ImmSetConversionStatus from nit: Gets https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, nit: ConvertInputModeToImmFlags https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, This can be static method. And how about simply converting |initial_conversion_mode| into |new_conversion_mode| instead of returning the bit masks? void ConvertInputModeToImmFlags(ui::TextInputMode input_mode, DWORD initial_conversion_mode, DWORD* new_conversion_mode, BOOL* open); https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:249: // Set conversion status corresponding to |input_mode|. nit: Sets https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:9: namespace { How about putting all the stuff (including unit test itself) into anonymous namespace? e.g. namespace ui { namespace { struct InputModeTestCase {...} class IMM32ManagerTest... const InputModeTestCase kInputModeTestCases[] = { ... }; TEST_P(...) INSTANTIATE_TEST_CASE_P(...) } // namespace } // namespace ui https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:27: {TEXT_INPUT_MODE_DEFAULT, 0,FALSE, 0}, nit: a white space is missing after 0, https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:27: {TEXT_INPUT_MODE_DEFAULT, 0,FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:28: {TEXT_INPUT_MODE_VERBATIM, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:49: {TEXT_INPUT_MODE_KATAKANA, We might want to have some additional test cases to make sure other bit flags are preserved. For instance, when Hiragana-mode is enabled with Romaji-style, the test case should be as follows. {TEXT_INPUT_MODE_KANA, IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN, TRUE, IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN}, (We should preserve IME_CMODE_ROMAN in this case.) https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:53: {TEXT_INPUT_MODE_NUMERIC, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:54: {TEXT_INPUT_MODE_TEL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:55: {TEXT_INPUT_MODE_EMAIL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:56: {TEXT_INPUT_MODE_URL, 0, FALSE, 0}, Please add a test case against |conversion_mode| is non-zero. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:90: ::testing::ValuesIn( I guess we can simply write ::testing::ValuesIn(kInputModeTestCases)
https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.h (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:243: // Get parameters for ::ImmSetOpenStatus and ::ImmSetConversionStatus from On 2013/07/26 04:06:05, Yohei Yukawa wrote: > nit: Gets Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > nit: ConvertInputModeToImmFlags Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:245: void ConvertInputModetoImmFlags(ui::TextInputMode input_mode, BOOL* open, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > nit: ConvertInputModeToImmFlags Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:249: // Set conversion status corresponding to |input_mode|. On 2013/07/26 04:06:05, Yohei Yukawa wrote: > nit: Sets Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:9: namespace { On 2013/07/26 04:06:05, Yohei Yukawa wrote: > How about putting all the stuff (including unit test itself) into anonymous > namespace? > > e.g. > > namespace ui { > namespace { > > struct InputModeTestCase {...} > > class IMM32ManagerTest... > > const InputModeTestCase kInputModeTestCases[] = { ... }; > > TEST_P(...) > > INSTANTIATE_TEST_CASE_P(...) > > } // namespace > } // namespace ui Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:27: {TEXT_INPUT_MODE_DEFAULT, 0,FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > nit: a white space is missing after 0, Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:28: {TEXT_INPUT_MODE_VERBATIM, 0, FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > Please add a test case against |conversion_mode| is non-zero. Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:49: {TEXT_INPUT_MODE_KATAKANA, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > We might want to have some additional test cases to make sure > other bit flags are preserved. For instance, when Hiragana-mode > is enabled with Romaji-style, the test case should be as follows. > > {TEXT_INPUT_MODE_KANA, > IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN, > TRUE, > IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE | IME_CMODE_ROMAN}, > (We should preserve IME_CMODE_ROMAN in this case.) Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:53: {TEXT_INPUT_MODE_NUMERIC, 0, FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > Please add a test case against |conversion_mode| is non-zero. Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:54: {TEXT_INPUT_MODE_TEL, 0, FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > Please add a test case against |conversion_mode| is non-zero. Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:55: {TEXT_INPUT_MODE_EMAIL, 0, FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > Please add a test case against |conversion_mode| is non-zero. Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:56: {TEXT_INPUT_MODE_URL, 0, FALSE, 0}, On 2013/07/26 04:06:05, Yohei Yukawa wrote: > Please add a test case against |conversion_mode| is non-zero. Done. https://chromiumcodereview.appspot.com/19690005/diff/22001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:90: ::testing::ValuesIn( On 2013/07/26 04:06:05, Yohei Yukawa wrote: > I guess we can simply write > ::testing::ValuesIn(kInputModeTestCases) Done.
https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager.cc (right): https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.cc:523: return; nit: indent https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager_unittest.cc:37: TRUE, Is it OK to turn on IME? https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager_unittest.cc:41: TRUE, Is it OK to turn on IME? https://codereview.chromium.org/19690005/diff/29002/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager_unittest.cc:45: TRUE, Is it OK to turn on IME?
https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.cc:523: return; On 2013/07/26 06:38:49, Yohei Yukawa wrote: > nit: indent Done. https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager_unittest.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... ui/base/ime/win/imm32_manager_unittest.cc:37: TRUE, On 2013/07/26 06:38:49, Yohei Yukawa wrote: > Is it OK to turn on IME? Yes. http://www.w3.org/html/wg/drafts/html/master/single-page.html#input-modalitie... HTML5 specifies that each inputmode="latin*" suggests to input latin text by user's IME.
> https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... > ui/base/ime/win/imm32_manager_unittest.cc:37: TRUE, > On 2013/07/26 06:38:49, Yohei Yukawa wrote: > > Is it OK to turn on IME? > > Yes. > http://www.w3.org/html/wg/drafts/html/master/single-page.html#input-modalitie... > HTML5 specifies that each inputmode="latin*" suggests to input latin text by > user's IME. That is still a draft and not a spec yet. Could you leave a comment about this? This behavior sounds a bit strange to me.
On 2013/07/26 07:07:09, Yohei Yukawa wrote: > > > https://chromiumcodereview.appspot.com/19690005/diff/29002/ui/base/ime/win/im... > > ui/base/ime/win/imm32_manager_unittest.cc:37: TRUE, > > On 2013/07/26 06:38:49, Yohei Yukawa wrote: > > > Is it OK to turn on IME? > > > > Yes. > > > http://www.w3.org/html/wg/drafts/html/master/single-page.html#input-modalitie... > > HTML5 specifies that each inputmode="latin*" suggests to input latin text by > > user's IME. > > That is still a draft and not a spec yet. Could you leave a comment about this? > This behavior sounds a bit strange to me. Add comment. If you use mozc, it is good to use IME suggestion, I think.
lgtm lgtm
lgtm
lgtm
ping nona@?
lgtm lgtm, mostly style nits. https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager.cc (right): https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.cc:526: BOOL open; please initialize. https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.cc:531: if (open) { you can drop braces. https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... File ui/base/ime/win/imm32_manager.h (right): https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.h:244: void SetTextInputMode(HWND window_handle, ui::TextInputMode input_mode); nit: seems no need ui:: https://codereview.chromium.org/19690005/diff/65001/ui/base/ime/win/imm32_man... ui/base/ime/win/imm32_manager.h:258: // |input_mode| should end with period
Thanks! https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.cc (right): https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.cc:526: BOOL open; On 2013/07/30 20:46:45, Seigo Nonaka wrote: > please initialize. Done. https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.cc:531: if (open) { On 2013/07/30 20:46:45, Seigo Nonaka wrote: > you can drop braces. Done. https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... File ui/base/ime/win/imm32_manager.h (right): https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:244: void SetTextInputMode(HWND window_handle, ui::TextInputMode input_mode); On 2013/07/30 20:46:45, Seigo Nonaka wrote: > nit: seems no need ui:: Done. https://chromiumcodereview.appspot.com/19690005/diff/65001/ui/base/ime/win/im... ui/base/ime/win/imm32_manager.h:258: // |input_mode| On 2013/07/30 20:46:45, Seigo Nonaka wrote: > should end with period Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoichio@chromium.org/19690005/75001
Message was sent while issue was closed.
Change committed as 214577 |