Fix how the Enter action is decided

This unifies the software and hardware keyboard code
under a single decision process that works.

Bug: 8129303
Bug: 8152758
Change-Id: I7574c563d5f957d57bfe62fe5e3eec59a519d335

java/res/xml/key_styles_enter.xml

@@ -104,7 +104,7 @@
         latin:parentStyle="defaultEnterKeyStyle" />
     <key-style
         latin:styleName="defaultActionEnterKeyStyle"
-        latin:code="!code/key_action_enter"
+        latin:code="!code/key_enter"
         latin:keyIcon="!icon/undefined"
         latin:backgroundType="action"
         latin:parentStyle="defaultEnterKeyStyle" />

java/src/com/android/inputmethod/accessibility/KeyCodeDescriptionMapper.java

@@ -110,7 +110,9 @@ public final class KeyCodeDescriptionMapper {
             return getDescriptionForShiftKey(context, keyboard);
         }
 
-        if (code == Constants.CODE_ACTION_ENTER) {
+        if (code == Constants.CODE_ENTER) {
+            // The following function returns the correct description in all action and
+            // regular enter cases, taking care of all modes.
             return getDescriptionForActionKey(context, keyboard, key);
         }
 

java/src/com/android/inputmethod/event/HardwareKeyboardEventDecoder.java

@@ -58,11 +58,11 @@ public class HardwareKeyboardEventDecoder implements HardwareEventDecoder {
             }
             if (KeyEvent.KEYCODE_ENTER == keyCode) {
                 // The Enter key. If the Shift key is not being pressed, this should send a
-                // CODE_ACTION_ENTER to trigger the action if any, or a carriage return
+                // CODE_ENTER to trigger the action if any, or a carriage return otherwise. If the
-                // otherwise. If the Shift key is depressed, this should send a
+                // Shift key is being pressed, this should send a CODE_SHIFT_ENTER and let
-                // CODE_SHIFT_ENTER and let Latin IME decide what to do with it.
+                // Latin IME decide what to do with it.
                 return Event.createCommittableEvent(keyEvent.isShiftPressed()
-                        ? Constants.CODE_SHIFT_ENTER : Constants.CODE_ACTION_ENTER,
+                        ? Constants.CODE_SHIFT_ENTER : Constants.CODE_ENTER,
                         null /* next */);
             }
             // If not Enter, then we have a committable character. This should be committed

java/src/com/android/inputmethod/keyboard/internal/KeyboardCodesSet.java

@@ -48,7 +48,6 @@ public final class KeyboardCodesSet {
         "key_delete",
         "key_settings",
         "key_shortcut",
-        "key_action_enter",
         "key_action_next",
         "key_action_previous",
         "key_shift_enter",
@@ -85,7 +84,6 @@ public final class KeyboardCodesSet {
         Constants.CODE_DELETE,
         Constants.CODE_SETTINGS,
         Constants.CODE_SHORTCUT,
-        Constants.CODE_ACTION_ENTER,
         Constants.CODE_ACTION_NEXT,
         Constants.CODE_ACTION_PREVIOUS,
         Constants.CODE_SHIFT_ENTER,
@@ -118,7 +116,6 @@ public final class KeyboardCodesSet {
         DEFAULT[12],
         DEFAULT[13],
         DEFAULT[14],
-        DEFAULT[15],
         CODE_RIGHT_PARENTHESIS,
         CODE_LEFT_PARENTHESIS,
         CODE_GREATER_THAN_SIGN,
@@ -142,7 +139,7 @@ public final class KeyboardCodesSet {
     };
 
     static {
-        if (DEFAULT.length != RTL.length) {
+        if (DEFAULT.length != RTL.length || DEFAULT.length != ID_TO_NAME.length) {
             throw new RuntimeException("Internal inconsistency");
         }
         for (int i = 0; i < ID_TO_NAME.length; i++) {

java/src/com/android/inputmethod/latin/Constants.java

@@ -179,14 +179,13 @@ public final class Constants {
     public static final int CODE_DELETE = -4;
     public static final int CODE_SETTINGS = -5;
     public static final int CODE_SHORTCUT = -6;
-    public static final int CODE_ACTION_ENTER = -7;
+    public static final int CODE_ACTION_NEXT = -7;
-    public static final int CODE_ACTION_NEXT = -8;
+    public static final int CODE_ACTION_PREVIOUS = -8;
-    public static final int CODE_ACTION_PREVIOUS = -9;
+    public static final int CODE_LANGUAGE_SWITCH = -9;
-    public static final int CODE_LANGUAGE_SWITCH = -10;
+    public static final int CODE_RESEARCH = -10;
-    public static final int CODE_RESEARCH = -11;
+    public static final int CODE_SHIFT_ENTER = -11;
-    public static final int CODE_SHIFT_ENTER = -12;
     // Code value representing the code is not specified.
-    public static final int CODE_UNSPECIFIED = -13;
+    public static final int CODE_UNSPECIFIED = -12;
 
     public static boolean isLetterCode(final int code) {
         return code >= CODE_SPACE;
@@ -200,7 +199,6 @@ public final class Constants {
         case CODE_DELETE: return "delete";
         case CODE_SETTINGS: return "settings";
         case CODE_SHORTCUT: return "shortcut";
-        case CODE_ACTION_ENTER: return "actionEnter";
         case CODE_ACTION_NEXT: return "actionNext";
         case CODE_ACTION_PREVIOUS: return "actionPrevious";
         case CODE_LANGUAGE_SWITCH: return "languageSwitch";

java/src/com/android/inputmethod/latin/InputTypeUtils.java

@@ -106,18 +106,13 @@ public final class InputTypeUtils implements InputType {
     }
 
     public static int getImeOptionsActionIdFromEditorInfo(final EditorInfo editorInfo) {
-        final int actionId = editorInfo.imeOptions & EditorInfo.IME_MASK_ACTION;
         if ((editorInfo.imeOptions & EditorInfo.IME_FLAG_NO_ENTER_ACTION) != 0) {
             return EditorInfo.IME_ACTION_NONE;
         } else if (editorInfo.actionLabel != null) {
             return IME_ACTION_CUSTOM_LABEL;
         } else {
-            return actionId;
+            // Note: this is different from editorInfo.actionId, hence "ImeOptionsActionId"
+            return editorInfo.imeOptions & EditorInfo.IME_MASK_ACTION;
         }
     }
-
-    public static int getConcreteActionIdFromEditorInfo(final EditorInfo editorInfo) {
-        final int actionId = getImeOptionsActionIdFromEditorInfo(editorInfo);
-        return actionId == InputTypeUtils.IME_ACTION_CUSTOM_LABEL ? editorInfo.actionId : actionId;
-    }
 }

java/src/com/android/inputmethod/latin/LatinIME.java

@@ -165,7 +165,6 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
     private boolean mExpectingUpdateSelection;
     private int mDeleteCount;
     private long mLastKeyTime;
-    private int mActionId;
     private TreeSet<Long> mCurrentlyPressedHardwareKeys = CollectionUtils.newTreeSet();
 
     // Member variables for remembering the current device orientation.
@@ -756,7 +755,6 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
 
         mLastSelectionStart = editorInfo.initialSelStart;
         mLastSelectionEnd = editorInfo.initialSelEnd;
-        mActionId = InputTypeUtils.getConcreteActionIdFromEditorInfo(editorInfo);
 
         mHandler.cancelUpdateSuggestionStrip();
         mHandler.cancelDoubleSpacePeriodTimer();
@@ -1393,13 +1391,28 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
                 ResearchLogger.getInstance().onResearchKeySelected(this);
             }
             break;
-        case Constants.CODE_ACTION_ENTER:
+        case Constants.CODE_ENTER:
-            if (EditorInfo.IME_ACTION_NONE != mActionId
+            final EditorInfo editorInfo = getCurrentInputEditorInfo();
-                && EditorInfo.IME_ACTION_UNSPECIFIED != mActionId) {
+            final int imeOptionsActionId =
-                performEditorAction(mActionId);
+                    InputTypeUtils.getImeOptionsActionIdFromEditorInfo(editorInfo);
-                break;
+            if (InputTypeUtils.IME_ACTION_CUSTOM_LABEL == imeOptionsActionId) {
+                // Either we have an actionLabel and we should performEditorAction with actionId
+                // regardless of its value.
+                performEditorAction(editorInfo.actionId);
+            } else if (EditorInfo.IME_ACTION_NONE != imeOptionsActionId) {
+                // We didn't have an actionLabel, but we had another action to execute.
+                // EditorInfo.IME_ACTION_NONE explicitly means no action. In contrast,
+                // EditorInfo.IME_ACTION_UNSPECIFIED is the default value for an action, so it
+                // means there should be an action and the app didn't bother to set a specific
+                // code for it - presumably it only handles one. It does not have to be treated
+                // in any specific way: anything that is not IME_ACTION_NONE should be sent to
+                // performEditorAction.
+                performEditorAction(imeOptionsActionId);
+            } else {
+                // No action label, and the action from imeOptions is NONE: this is a regular
+                // enter key that should input a carriage return.
+                didAutoCorrect = handleNonSpecialCharacter(Constants.CODE_ENTER, x, y, spaceState);
             }
-            didAutoCorrect = handleNonSpecialCharacter(Constants.CODE_ENTER, x, y, spaceState);
             break;
         case Constants.CODE_SHIFT_ENTER:
             didAutoCorrect = handleNonSpecialCharacter(Constants.CODE_ENTER, x, y, spaceState);