Generate key event for deleteSurroundingText

Generate key event for deleteSurroundingText

Currently Chrome on android does not generate any key event when BACKSPACE key
is pressed outside of composition, in other words, when AdapterInputConnection.
deleteSurroundingText is called. It breaks websites who expects to always
receive a KeyDown/KeyUp event when BACKSPACE key is pressed.

This CL fixes the issue by generating key events in deleteSurroundingText.

For more details please refer to the attached bug.


BUG=184812
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279928

[guohui, 2014-06-23 23:02:41]

Hey,

could you please review the CL?

Thanks,

Hui

[aurimas (slooooooooow), 2014-06-24 23:03:03]

https://codereview.chromium.org/348413003/diff/60001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/348413003/diff/60001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:331:
mImeAdapter.sendKeyEventWithKeyCode(KeyEvent.KEYCODE_UNKNOWN,
I think we should move this additional logic to ImeAdapter.java

Could we do something similar like we do for enter and tab in
ImeAdapter#shouldSendKeyEventWithKeyCode(): use nativeSendSyntheticKeyEvent to
send a synthetic key event with a delete keycode.

[guohui, 2014-06-25 17:50:33]

https://codereview.chromium.org/348413003/diff/60001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/348413003/diff/60001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:331:
mImeAdapter.sendKeyEventWithKeyCode(KeyEvent.KEYCODE_UNKNOWN,
On 2014/06/24 23:03:03, aurimas wrote:
> I think we should move this additional logic to ImeAdapter.java
> 
> Could we do something similar like we do for enter and tab in
> ImeAdapter#shouldSendKeyEventWithKeyCode(): use nativeSendSyntheticKeyEvent to
> send a synthetic key event with a delete keycode.

Moved the logic to ImeAdapter as suggested.

However we cannot send the delete keycode, as i mentioned in the comment above,
sending DEL keycode will cause Chrome to delete one extra char at the end. E.g.
if the current text is "hello " and the caret is at the end, sending DEL keycode
with deleteSurroundingText will delete both 'o' and ' ' instead of only the ' '.
I suspect it requires non-trivial refactoring to trigger JS listeners without
any side effect, thus i added a comment here and will handle it in a future CL.

[guohui, 2014-06-25 19:52:40]

ping ted@ for owner review?

[guohui, 2014-06-25 19:52:41]

ping ted@ for owner review?

[aurimas (slooooooooow), 2014-06-25 19:54:27]

LGTM after discussion offline that we will try to fix it differently after IME
vs Key event race conditions are addressed.

[aurimas (slooooooooow), 2014-06-25 20:22:38]

On 2014/06/25 19:54:27, aurimas wrote:
> LGTM after discussion offline that we will try to fix it differently after IME
> vs Key event race conditions are addressed.

Just FYI, I am an owner for this directory.

[guohui, 2014-06-25 20:28:15]

On 2014/06/25 20:22:38, aurimas wrote:
> On 2014/06/25 19:54:27, aurimas wrote:
> > LGTM after discussion offline that we will try to fix it differently after
IME
> > vs Key event race conditions are addressed.
> 
> Just FYI, I am an owner for this directory.

great, missed that =)

[Ted C, 2014-06-25 20:41:51]

one quick nit (still lgtm since Aurimas gave it one).

https://chromiumcodereview.appspot.com/348413003/diff/80001/content/public/an...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://chromiumcodereview.appspot.com/348413003/diff/80001/content/public/an...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:413:
KeyEvent.FLAG_SOFT_KEYBOARD | KeyEvent.FLAG_KEEP_TOUCH_MODE);
+4 indent

[aurimas (slooooooooow), 2014-06-25 23:00:57]

still lgtm

[guohui, 2014-06-25 23:01:37]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-06-25 23:05:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/guohui@chromium.org/348413003/160001

[commit-bot: I haz the power, 2014-06-26 06:28:18]

Change committed as 279928