Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(144)

Issue 2255002: When the previous engine name is unknown, switch to the next engine in menu. (Closed)

Created:
10 years, 7 months ago by satorux1
Modified:
9 years, 7 months ago
Reviewers:
James Su
CC:
chromium-os-reviews_chromium.org, Yusuke Sato
Base URL:
ssh://git@gitrw.chromium.org//ibus.git
Visibility:
Public.

Description

When the previous engine name is unknown, switch to the next engine in menu. Before the change, hitting the "previous-engine" key does nothing if the previous engine name is unknown. The behavior is not good for users as they think the hot key is not working. TEST=manually on the netbook BUG=3579

Patch Set 1 #

Patch Set 2 : remove stdio #

Total comments: 5

Patch Set 3 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -0 lines) Patch
M bus/ibusimpl.c View 1 2 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
satorux1
10 years, 7 months ago (2010-05-26 08:06:02 UTC) #1
James Su
See comments below. http://codereview.chromium.org/2255002/diff/3001/4001 File bus/ibusimpl.c (right): http://codereview.chromium.org/2255002/diff/3001/4001#newcode984 bus/ibusimpl.c:984: if (!engine_name && g_list_length(ibus->engine_list) > 1) ...
10 years, 7 months ago (2010-05-26 08:18:11 UTC) #2
satorux1
James, thank you for the fast response! http://codereview.chromium.org/2255002/diff/3001/4001 File bus/ibusimpl.c (right): http://codereview.chromium.org/2255002/diff/3001/4001#newcode984 bus/ibusimpl.c:984: if (!engine_name ...
10 years, 7 months ago (2010-05-26 08:29:47 UTC) #3
James Su
10 years, 7 months ago (2010-05-26 08:32:53 UTC) #4
LGTM.

http://codereview.chromium.org/2255002/diff/3001/4001
File bus/ibusimpl.c (right):

http://codereview.chromium.org/2255002/diff/3001/4001#newcode988
bus/ibusimpl.c:988: _context_request_engine_cb (context, engine_name, ibus);
On 2010/05/26 08:29:48, satorux1 wrote:
> On 2010/05/26 08:18:12, James Su wrote:
> > We may change this function to check if the new engine_name is same as the
> > current one. If they are same then just enable the context.
> 
> I think we don't have to do it in this patch, right?

Right. So just leave it as it.

Powered by Google App Engine
This is Rietveld 408576698