Issue 80583002: [FYI] All-in-one OnCandidateWindow{Show,Update,Hide}

Issue 80583002: [FYI] All-in-one OnCandidateWindow{Show,Update,Hide} (Closed)

Created:
7 years, 1 month ago by kochi

Modified:
7 years ago

Reviewers:
yukawa

CC:
chromium-reviews, jbauman+watch_chromium.org, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, tfarina, jam, penghuang+watch_chromium.org, sievers+watch_chromium.org, joi+watch-content_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, kalyank, piman+watch_chromium.org, danakj+watch_chromium.org, James Su, miu+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

[FYI] All-in-one OnCandidateWindow{Show,Update,Hide} Chromium side of implementation for IME API's oncandidatewindow* events. BUG=238585

Patch Set 1 #

Total comments: 3

Patch Set 2 : rebase & apply other CL's updates #

Patch Set 3 : posttask #

Patch Set 4 : rebase #

Patch Set 5 : move posttask, weakptr #

Patch Set 6 : . #

Patch Set 7 : . #

Patch Set 8 : rebase #

Patch Set 9 : move posttask #

Patch Set 10 : unittest #

Patch Set 11 : update #

Created: 7 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+243 lines, -7 lines)			Patch
M	ui/base/ime/input_method_base.h	View	1 2 3 4 5 6 7 8 9 10	4 chunks	+18 lines, -1 line	0 comments	Download
M	ui/base/ime/input_method_base.cc	View	1 2 3 4 5 6 7 8 9 10	2 chunks	+36 lines, -0 lines	0 comments	Download
M	ui/base/ime/input_method_base_unittest.cc	View	1 2 3 4 5 6 7 8 9 10	6 chunks	+83 lines, -4 lines	0 comments	Download
M	ui/base/ime/input_method_imm32.cc	View	1 2 3 4 5 6 7 8	1 chunk	+12 lines, -0 lines	0 comments	Download
M	ui/base/ime/input_method_tsf.cc	View	1 2 3 4 5 6 7 8 9 10	2 chunks	+12 lines, -1 line	0 comments	Download
M	ui/base/ime/win/mock_tsf_bridge.h	View	1 2 3 4	1 chunk	+3 lines, -0 lines	0 comments	Download
M	ui/base/ime/win/mock_tsf_bridge.cc	View	1 2 3 4 5 6	1 chunk	+9 lines, -0 lines	0 comments	Download
M	ui/base/ime/win/tsf_bridge.h	View	1 2 3 4 5 6 7 8 9 10	1 chunk	+5 lines, -0 lines	0 comments	Download
M	ui/base/ime/win/tsf_bridge.cc	View	1 2 3 4 5 6 7 8 9 10	5 chunks	+56 lines, -1 line	0 comments	Download
M	ui/views/ime/input_method_bridge.cc	View	1 2 3	1 chunk	+9 lines, -0 lines	0 comments	Download

Messages

Total messages: 1 (0 generated)

Expand Messages | Collapse Messages

yukawa

7 years ago (2013-11-25 05:07:41 UTC) #1

Just an early feedback.

https://codereview.chromium.org/80583002/diff/1/ui/base/ime/input_method_imm3...
File ui/base/ime/input_method_imm32.cc (right):

https://codereview.chromium.org/80583002/diff/1/ui/base/ime/input_method_imm3...
ui/base/ime/input_method_imm32.cc:241: NotifyCandidateWindowStateChanged();
I'm not sure if we can safely call back external code from here.  Using PostTask
to avoid nested message handler might be safer.  Please note that objects that
implement TextInputClient can do anything (e.g., calling
InputMehotd::CancelComposition) at this line.

https://codereview.chromium.org/80583002/diff/1/ui/base/ime/input_method_imm3...
ui/base/ime/input_method_imm32.cc:249: // while this may fire more often without
window resize. There is no generic
"There is no generic way to get bounds of candidate window."
I haven't investigated yet but we might be able to use IMC_GETCANDIDATEPOS
request. If this is true, we might want to
say "Not all IMEs support IMC_GETCANDIDATEPOS request."

https://codereview.chromium.org/80583002/diff/1/ui/base/ime/input_method_tsf.cc
File ui/base/ime/input_method_tsf.cc (right):

https://codereview.chromium.org/80583002/diff/1/ui/base/ime/input_method_tsf....
ui/base/ime/input_method_tsf.cc:25:
ui::TSFBridge::GetInstance()->OnCandidateWindowShow();
I'm not sure if we can safely call back external code from here.  Using PostTask
to avoid nested message handler might be safer.  Please note that objects that
implement TextInputClient can do anything (e.g., calling
InputMehotd::CancelComposition) at this line.

Expand Messages | Collapse Messages