[Android] Adding Smart GO/NEXT feature in Chrome

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

Index: content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
index 49726776c49baedb8e0e14ee3e365a438b160e87..3f83ecfc216e49da48d3899aa2345d0203750b54 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
@@ -49,7 +49,6 @@ public class ImeUtils {
         if (inputMode == WebTextInputMode.DEFAULT) {
             if (inputType == TextInputType.TEXT) {
                 // Normal text field
-                imeAction = EditorInfo.IME_ACTION_GO;
                 if ((inputFlags & WebTextInputFlags.AUTOCORRECT_OFF) == 0) {
                     outAttrs.inputType |= EditorInfo.TYPE_TEXT_FLAG_AUTO_CORRECT;
                 }
@@ -59,33 +58,27 @@ public class ImeUtils {
                 if ((inputFlags & WebTextInputFlags.AUTOCORRECT_OFF) == 0) {
                     outAttrs.inputType |= EditorInfo.TYPE_TEXT_FLAG_AUTO_CORRECT;
                 }
-                imeAction = EditorInfo.IME_ACTION_NONE;
             } else if (inputType == TextInputType.PASSWORD) {
                 outAttrs.inputType =
                         InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_WEB_PASSWORD;
-                imeAction = EditorInfo.IME_ACTION_GO;
             } else if (inputType == TextInputType.SEARCH) {
                 imeAction = EditorInfo.IME_ACTION_SEARCH;
             } else if (inputType == TextInputType.URL) {
                 outAttrs.inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_URI;
-                imeAction = EditorInfo.IME_ACTION_GO;
             } else if (inputType == TextInputType.EMAIL) {
                 // Email
                 outAttrs.inputType =
                         InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_VARIATION_WEB_EMAIL_ADDRESS;
-                imeAction = EditorInfo.IME_ACTION_GO;
             } else if (inputType == TextInputType.TELEPHONE) {
                 // Telephone
                 // Number and telephone do not have both a Tab key and an
                 // action in default OSK, so set the action to NEXT
                 outAttrs.inputType = InputType.TYPE_CLASS_PHONE;
-                imeAction = EditorInfo.IME_ACTION_NEXT;
             } else if (inputType == TextInputType.NUMBER) {
                 // Number
                 outAttrs.inputType = InputType.TYPE_CLASS_NUMBER
                         | InputType.TYPE_NUMBER_VARIATION_NORMAL
                         | InputType.TYPE_NUMBER_FLAG_DECIMAL;
-                imeAction = EditorInfo.IME_ACTION_NEXT;
             }
         } else {
             switch (inputMode) {
@@ -128,6 +121,34 @@ public class ImeUtils {
         }
 
         outAttrs.imeOptions |= imeAction;
+
+        boolean enterKeyAction = false;

[Changwan Ryu, 2017/04/27 16:34:53]

1. You need to put this in front of switch statement above, because inputMode
should be able to override implicit ime actions.

2. Instead of keeping local variables, how about the following structure?

if () {
    // Comment
    ...
} else if () {
    // Comment
    ...
} else if () {
    // Comment
    ...
}

Or better yet, you could refactor this into a small function such as:

int getImeActions(int inputType, int inputMode, boolean
listeningToKeyboardEvents) {
  // Input mode overrides implicit IME actions.
  switch(inputMode) {
      return ...;
  }

  if () {
      return ...;
  }
  if () {
      return ...;
  }
  if () {
      return ...;
  }
  return EditorInfo.IME_ACTION_NONE;
}

[AKVT, 2017/05/03 14:35:12]

You mean we should overwrite inputMode imeAction ? I also felt same, because fir
WebTextInputMode.TEL, WebTextInputMode.URL etc. it's by default keeping
ACTION_NEXT. But ideally not needed based on near by element presence as per
this logic. WDYT ?
I thought to have your comment before modifying existing switch (inputMode) {}
code. 

[Changwan Ryu, 2017/05/04 20:38:15]

My bad, I got confused about input mode. Input mode action should also be fully
replaced by the new ones, instead of OR-ing new values.

[AKVT, 2017/05/09 11:05:52]

Thanks. I corrected in PS7

+        // For multiline text or any text input with key event listeners, Enter key is needed.

[Changwan Ryu, 2017/04/27 16:34:54]

nit: ENTER (all cap) or enter (all small)

[AKVT, 2017/05/03 14:35:12]

Done.

+        if (((outAttrs.inputType & EditorInfo.TYPE_TEXT_FLAG_MULTI_LINE)
+                    == EditorInfo.TYPE_TEXT_FLAG_MULTI_LINE)

[Changwan Ryu, 2017/04/27 16:34:54]

instead of == you can use > 0 or != 0

[AKVT, 2017/05/03 14:35:11]

Done.

+                || (inputFlags & WebTextInputFlags.LISTENING_TO_KEYBOARD_EVENTS) != 0) {
+            // TODO(ajith.v) Find an option to remove Enter Key from IME (assume by default

[Changwan Ryu, 2017/04/27 16:34:54]

No need to have a TODO on our end. Most likely IME's decision and/or ecosystem
issue.

[AKVT, 2017/05/03 14:35:12]

Done.

+            // ENTER key is shown)
+            // TODO(ajith.v) For text area with implicit form submission how do we handle,

[Changwan Ryu, 2017/04/27 16:34:54]

By implicit form submission, do you mean JavaScript key handling? If so,
IME_ACTION_NONE should be enough, and you can instead leave a comment such as:

For textarea that sends you to another webpage on enter key press using
JavaScript, we will only show ENTER.

[AKVT, 2017/05/03 14:35:12]

Yes implicit submission I meant with that only. On ENTER key press, it will
submit the form instead of adding a new line (Very rare content though). But if
we have both the keys (ENTER and GO), then we can achieve both. (New line on
ENTER key and form submission on GO). WDYT ?

[Changwan Ryu, 2017/05/04 20:38:15]

multiline --> always ENTER
single line --> always GO/NEXT/PREV

what's wrong with this?

[AKVT, 2017/05/09 11:05:52]

Yes currently I have done like this. Also NEXT/GO/PREV I am setting individually
because multiple ORing has issues in display and is executing only one action at
a time, because it's sharing with one key space.

+            // we need to show both GO and ENTER key at same time.
+            outAttrs.imeOptions |= EditorInfo.IME_ACTION_GO;
+            enterKeyAction = true;
+        }
+
+        boolean nextKeyAction = false;
+        // If ENTER key is already applied, then don't apply NEXT as it can result display issues in
+        // IME
+        if (!enterKeyAction && (inputFlags & WebTextInputFlags.HAVE_NEXT_FOCUSABLE_ELEMENT) != 0) {
+            outAttrs.imeOptions |= EditorInfo.IME_ACTION_NEXT;
+            nextKeyAction = true;
+        }
+        // If ENTER/NEXT key is already applied, then don't apply PREVIOUS as it can result display
+        // issues in IME

[Changwan Ryu, 2017/04/27 16:34:54]

This is quite unfortunate. Could you file/refer to a crbug or be specific about
on which IMEs you see this?

[AKVT, 2017/05/03 14:35:12]

Currently I am using Google Pixel device default Google IME for my testing.

In fact this problem I have noticed long back on Samsung device IME as well.

AFAIU we may have to file this bug to Google IME team.

[Changwan Ryu, 2017/05/04 20:38:15]

Could you leave those keyboards (not the device) as examples in the comment?

+        if (!enterKeyAction && !nextKeyAction
+                && (inputFlags & WebTextInputFlags.HAVE_PREVIOUS_FOCUSABLE_ELEMENT) != 0) {
+            outAttrs.imeOptions |= EditorInfo.IME_ACTION_PREVIOUS;
+        }
+
         // Handling of autocapitalize. Blink will send the flag taking into account the element's
         // type. This is not using AutocapitalizeNone because Android does not autocapitalize by
         // default and there is no way to express no capitalization.