Translate physical keyboard accents to IME compositions.

content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.content.browser.input;
 
 import android.os.SystemClock;
 import android.text.Editable;
 import android.text.InputType;
 import android.text.Selection;
 import android.text.TextUtils;
 import android.util.Log;
+import android.view.KeyCharacterMap;
 import android.view.KeyEvent;
 import android.view.View;
 import android.view.inputmethod.BaseInputConnection;
 import android.view.inputmethod.EditorInfo;
 import android.view.inputmethod.ExtractedText;
 import android.view.inputmethod.ExtractedTextRequest;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.blink_public.web.WebInputEventType;
 import org.chromium.blink_public.web.WebTextInputFlags;
@@ skipping 12 matching lines @@
      */
     public static final int INVALID_SELECTION = -1;
     public static final int INVALID_COMPOSITION = -1;
 
     private final View mInternalView;
     private final ImeAdapter mImeAdapter;
     private final Editable mEditable;
 
     private boolean mSingleLine;
     private int mNumNestedBatchEdits = 0;
+    private int mPendingAccent;
 
     private int mLastUpdateSelectionStart = INVALID_SELECTION;
     private int mLastUpdateSelectionEnd = INVALID_SELECTION;
     private int mLastUpdateCompositionStart = INVALID_COMPOSITION;
     private int mLastUpdateCompositionEnd = INVALID_COMPOSITION;
 
     @VisibleForTesting
     AdapterInputConnection(View view, ImeAdapter imeAdapter, Editable editable,
             EditorInfo outAttrs) {
         super(view, true);
@@ skipping 180 matching lines @@
         mLastUpdateCompositionEnd = compositionEnd;
     }
 
     /**
      * @see BaseInputConnection#setComposingText(java.lang.CharSequence, int)
      */
     @Override
     public boolean setComposingText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "setComposingText [" + text + "] [" + newCursorPosition + "]");
         if (maybePerformEmptyCompositionWorkaround(text)) return true;
+        mPendingAccent = 0;
         super.setComposingText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, false);
     }
 
     /**
      * @see BaseInputConnection#commitText(java.lang.CharSequence, int)
      */
     @Override
     public boolean commitText(CharSequence text, int newCursorPosition) {
         if (DEBUG) Log.w(TAG, "commitText [" + text + "] [" + newCursorPosition + "]");
         if (maybePerformEmptyCompositionWorkaround(text)) return true;
+        mPendingAccent = 0;
         super.commitText(text, newCursorPosition);
         updateSelectionIfRequired();
         return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition,
                 text.length() > 0);
     }
 
     /**
      * @see BaseInputConnection#performEditorAction(int)
      */
     @Override
@@ skipping 77 matching lines @@
     @Override
     public boolean deleteSurroundingText(int beforeLength, int afterLength) {
         return deleteSurroundingTextImpl(beforeLength, afterLength, false);
     }
 
     private boolean deleteSurroundingTextImpl(
             int beforeLength, int afterLength, boolean fromPhysicalKey) {
         if (DEBUG) {
             Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]");
         }
+
+        if (mPendingAccent != 0) {
+            mPendingAccent = 0;
+            finishComposingText();
+        }
+
         int originalBeforeLength = beforeLength;
         int originalAfterLength = afterLength;
         int availableBefore = Selection.getSelectionStart(mEditable);
         int availableAfter = mEditable.length() - Selection.getSelectionEnd(mEditable);
         beforeLength = Math.min(beforeLength, availableBefore);
         afterLength = Math.min(afterLength, availableAfter);
         super.deleteSurroundingText(beforeLength, afterLength);
         updateSelectionIfRequired();
 
         // If this was called due to a physical key, no need to generate a key event here as
@@ skipping 32 matching lines @@
     @Override
     public boolean sendKeyEvent(KeyEvent event) {
         if (DEBUG) {
             Log.w(TAG, "sendKeyEvent [" + event.getAction() + "] [" + event.getKeyCode() + "]");
         }
 
         int action = event.getAction();
         int keycode = event.getKeyCode();
         int unicodeChar = event.getUnicodeChar();
 
+        // If we're not focused in a textbox or if this isn't a KeyDown event, no need to update
+        // composition state; just pass the key event through and return.
+        if (mImeAdapter.getTextInputType() == TextInputType.NONE
+                || action != KeyEvent.ACTION_DOWN) {

[aurimas (slooooooooow), 2015/06/03 00:44:20]

Would this result in us sending KeyUp events without corresponding KeyDown
events for keyevents with a combining accent? Do you foresee that being a
problem? I am assuming that we will fire KeyUp events in javascript, but now
KeyDown too.

[aelias_OOO_until_Jul13, 2015/06/03 01:15:01]

Yes, it does and I spent some time looking into if this could be avoided.  But,
the only two ways I can think to fix this are either storing state to remember
to suppress KeyUp (which is easy to get wrong and suppress the wrong thing
later), or write a completely new accent-specific implementation of
setComposingText() to avoid the additional KeyUp there, and both of those
changes seemed too disruptive to be worth it.  I observed accent keys on Mac
already do something similar (send 229 on Keydown but accent keycode on Keyup),
I think Javascript would rarely use KeyUp outside of game-style use cases, so
it's probably harmless in practice.

[bcwhite, 2015/06/03 15:28:58]

I've been trying this patch with a physical keyboard against
http://www.backgroundexposure.com/t/key-event-test.html and it's not working at
all.  My debugging indicates that TextInputType is staying as None and thus
avoiding the entire logic due to this short-circuit.

The core of it is that updateKeyboardVisibility() is being called with
showIfNeeded==false and the type is never being updated.  The type isn't really
important with a physical keyboard so it's never been a problem.

[aelias_OOO_until_Jul13, 2015/06/03 21:10:29]

Good catch, thanks.  It looks like this is a side effect of the logic in the
renderer to avoid bringing up the soft keyboard unless there's been a user
gesture.  I'll look at how to decouple that from this in the future, but as this
is for cherry-pick to 44 and the raw key event use case is minor, I just removed
the TextInputType early-return.

+            mImeAdapter.translateAndSendNativeEvents(event);
+            return true;
+        }
+
         // If this is backspace/del or if the key has a character representation,
         // need to update the underlying Editable (i.e. the local representation of the text
         // being edited).  Some IMEs like Jellybean stock IME and Samsung IME mix in delete
         // KeyPress events instead of calling deleteSurroundingText.
-        if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_DEL) {
+        if (keycode == KeyEvent.KEYCODE_DEL) {
             deleteSurroundingTextImpl(1, 0, true);
-        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_FORWARD_DEL) {
+        } else if (keycode == KeyEvent.KEYCODE_FORWARD_DEL) {

[aurimas (slooooooooow), 2015/06/03 00:44:20]

What would happen if we have DEL, FORWARD_DEL or ENTER key event between
combining accent and actual character events?

[aelias_OOO_until_Jul13, 2015/06/03 01:15:01]

I did try out those scenarios earlier today and have a unit test case covering
DEL.  The accent state and composition is terminated, and the accent will be
deleted (for DEL) or remain (for the others).   In both cases,
finishComposingText is called which also has "mPendingAccent = 0" in it.

             deleteSurroundingTextImpl(0, 1, true);
-        } else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_ENTER) {
+        } else if (keycode == KeyEvent.KEYCODE_ENTER) {
             // Finish text composition when pressing enter, as that may submit a form field.
             // TODO(aurimas): remove this workaround when crbug.com/278584 is fixed.
             beginBatchEdit();
             finishComposingText();
             mImeAdapter.translateAndSendNativeEvents(event);
             endBatchEdit();
             return true;
-        } else if (action == KeyEvent.ACTION_UP && unicodeChar != 0) {
+        } else if ((unicodeChar & KeyCharacterMap.COMBINING_ACCENT) != 0) {
+            // Store a pending accent character and make it the current composition.
+            int pendingAccent = unicodeChar & KeyCharacterMap.COMBINING_ACCENT_MASK;
+            StringBuilder builder = new StringBuilder();
+            builder.appendCodePoint(pendingAccent);
+            setComposingText(builder.toString(), 1);
+            mPendingAccent = pendingAccent;
+            return true;
+        }
+
+        if (unicodeChar != 0) {
+            if (mPendingAccent != 0) {
+                int combined = KeyEvent.getDeadChar(mPendingAccent, unicodeChar);
+                if (combined != 0) {
+                    StringBuilder builder = new StringBuilder();
+                    builder.appendCodePoint(combined);
+                    commitText(builder.toString(), 1);
+                    return true;
+                }
+                // Noncombinable character; commit the accent character and fall through to sending
+                // the key event for the character afterwards.
+                finishComposingText();
+            }
+
+            // Update the mEditable state to reflect what Blink will do in response to the KeyDown
+            // for a unicode-mapped key event.
             int selectionStart = Selection.getSelectionStart(mEditable);
             int selectionEnd = Selection.getSelectionEnd(mEditable);
             if (selectionStart > selectionEnd) {
                 int temp = selectionStart;
                 selectionStart = selectionEnd;
                 selectionEnd = temp;
             }
             mEditable.replace(selectionStart, selectionEnd,
                     Character.toString((char) unicodeChar));
         }
+
         mImeAdapter.translateAndSendNativeEvents(event);
         return true;
     }
 
     /**
      * @see BaseInputConnection#finishComposingText()
      */
     @Override
     public boolean finishComposingText() {
         if (DEBUG) Log.w(TAG, "finishComposingText");
         if (getComposingSpanStart(mEditable) == getComposingSpanEnd(mEditable)) {
             return true;
         }
 
+        mPendingAccent = 0;
         super.finishComposingText();
         updateSelectionIfRequired();
         mImeAdapter.finishComposingText();
 
         return true;
     }
 
     /**
      * @see BaseInputConnection#setSelection(int, int)
      */
     @Override
     public boolean setSelection(int start, int end) {
         if (DEBUG) Log.w(TAG, "setSelection [" + start + " " + end + "]");
         int textLength = mEditable.length();
         if (start < 0 || end < 0 || start > textLength || end > textLength) return true;
         super.setSelection(start, end);
         updateSelectionIfRequired();
         return mImeAdapter.setEditableSelectionOffsets(start, end);
     }
 
     /**
      * Informs the InputMethodManager and InputMethodSession (i.e. the IME) that the text
      * state is no longer what the IME has and that it needs to be updated.
      */
     void restartInput() {
         if (DEBUG) Log.w(TAG, "restartInput");
         getInputMethodManagerWrapper().restartInput(mInternalView);
         mNumNestedBatchEdits = 0;
+        mPendingAccent = 0;
     }
 
     /**
      * @see BaseInputConnection#setComposingRegion(int, int)
      */
     @Override
     public boolean setComposingRegion(int start, int end) {
         if (DEBUG) Log.w(TAG, "setComposingRegion [" + start + " " + end + "]");
         int textLength = mEditable.length();
         int a = Math.min(start, end);
@@ skipping 73 matching lines @@
     @VisibleForTesting
     ImeState getImeStateForTesting() {
         String text = mEditable.toString();
         int selectionStart = Selection.getSelectionStart(mEditable);
         int selectionEnd = Selection.getSelectionEnd(mEditable);
         int compositionStart = getComposingSpanStart(mEditable);
         int compositionEnd = getComposingSpanEnd(mEditable);
         return new ImeState(text, selectionStart, selectionEnd, compositionStart, compositionEnd);
     }
 }