New issue
Advanced search Search tips

Issue 641425 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

InputIme API on ChromeOS has very inconsistent return, error values

Project Member Reported by rdevlin....@chromium.org, Aug 26 2016

Issue description

The return and error values for the input ime api on chromeos are quite inconsistent, both with common extension API practice and within the API.

Many functions seem to be using a return value to indicate success or failure of a function, even when the failure would be more akin to an  error.  For instance, the clearComposition function (and others) return "false" when the extension doesn't have the active IME engine. [1]

Different functions, like setCandidates, return "true" for this same circumstance, even though it's clear the function didn't succeed. [2]

Others, in the case of the extension not having a registered engine, will in fact set runtime.lastError. [3]

Some functions can sometimes set the error_ field, but never check the result and always return indicating success from the function - which leaves the extension system wondering what to do (did the api call succeed or fail?) [4]  Similarly, some check whether a function fails and set the error field, but return true (indicating success) anyway. [5]  The latter seems to be masking a failing API test (or is an incorrect error).

[1] https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc#364
[2] https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc#464
[3] https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc#523
[4] https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc#581
[5] https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc#533

Ideally, we'd make all of these functions more consistent with common extension API practice and with each other. Unfortunately, it's unclear if anyone relies on this behavior, so we might not be able to change everything.
 
Cc: rdevlin....@chromium.org azurewei@chromium.org
Owner: shuchen@chromium.org
Status: Assigned (was: Unconfirmed)
Hey shuchen@, I noticed these in the course of https://codereview.chromium.org/2285573002/.  Can you look into them or pass them along?
Cc: -azurewei@chromium.org shuchen@chromium.org
Owner: azurewei@chromium.org
Most input.ime.* APIs on ChromeOS returns result (true/false) to indicates whether the method is properly run (as documented in https://developer.chrome.com/extensions/input_ime). The system build-in IMEs, as well as third-party IME extends rely on these behaviors. I think we'd better keep returning bool rather than setting runtime.lastError for now.
From those docs:
callback -

Called when the operation completes with a boolean indicating if the text was accepted or not. On failure, chrome.runtime.lastError is set.

If you specify the callback parameter, it should be a function that looks like this:

function(boolean success) {...};

This implies that both a return result of false *and* a runtime.lastError will be set.

Additionally, I agree that we may not be able to fix all of these, as I mentioned in #0.  But some of the behavior, like [5] from #0, is incorrect, regardless of how we indicate failure, since it returns "success" no matter what - even though it can fail.
Thanks for pointing the problems out. I've sent cl to make sure runtime.lastError will be set when the callback parameter returns false.
Cc: azurewei@chromium.org wuyingbing@chromium.org
 Issue 554445  has been merged into this issue.
Components: UI>Input
Components: -UI>Input UI>Input>Text>IME
Hi @shuchen @azurewei,

do we still have pending work on this issue? If not, can we close this?
Cc: chengong@chromium.org

Sign in to add a comment