Ensure input connection to be created on pressing next button

We have recently introduced a logic to return null input connection in View#onCreateInputConnection() to handle the physical keyboard case better. (https://codereview.chromium.org/1362603002) However, when the user is focused on a EditText and jumps to WebView using 'next' button on an input method, then the user cannot type a character because a real input connection was never created. WebView cannot distinguish the following two cases: - Gaining focus with autofocus=true (for which we intentionally do not show keyboard) - Gaining focus through 'next' button Before that CL, we were simply creating NONE type input connection at the beginning even when input form was not focused, which caused the original physical keyboard issue. In this CL, we will keep an input connection *whenever* currently focused node is editable, regardless of showIme value. Some tests should change accordingly: now input connection may not be null even when keyboard remains hidden. Also I'm adding a WebView test to prevent a regression. BUG=569556 Review URL: https://codereview.chromium.org/1534163002 Cr-Commit-Position: refs/heads/master@{#366583}

Ensure input connection to be created on pressing next button
We have recently introduced a logic to return null input connection in View#onCreateInputConnection() to handle the physical keyboard case better. (https://codereview.chromium.org/1362603002) However, when the user is focused on a EditText and jumps to WebView using 'next' button on an input method, then the user cannot type a character because a real input connection was never created. WebView cannot distinguish the following two cases: - Gaining focus with autofocus=true (for which we intentionally do not show keyboard) - Gaining focus through 'next' button Before that CL, we were simply creating NONE type input connection at the beginning even when input form was not focused, which caused the original physical keyboard issue. In this CL, we will keep an input connection *whenever* currently focused node is editable, regardless of showIme value. Some tests should change accordingly: now input connection may not be null even when keyboard remains hidden. Also I'm adding a WebView test to prevent a regression. BUG=569556 Review URL: https://codereview.chromium.org/1534163002 Cr-Commit-Position: refs/heads/master@{#366583}
945f5e29 · changwan · Commit bot · c05cb747 · 945f5e29 · 945f5e29
Commit 945f5e29 authored Dec 22, 2015 by changwan Committed by Commit bot Dec 22, 2015
3 changed files
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java
+// Copyright 2015 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.android_webview.test;
+
+import android.content.Context;
+import android.test.suitebuilder.annotation.SmallTest;
+import android.view.inputmethod.InputConnection;
+import android.view.inputmethod.InputMethodManager;
+import android.widget.EditText;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.Feature;
+import org.chromium.content.browser.test.util.CallbackHelper;
+import org.chromium.content.browser.test.util.Criteria;
+import org.chromium.content.browser.test.util.CriteriaHelper;
+
+/**
+ * Tests for IME (input method editor) on Android WebView.
+ */
+public class AwImeTest extends AwTestBase {
+
+    private TestAwContentsClient mContentsClient;
+    private AwTestContainerView mTestContainerView;
+    private EditText mEditText;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mContentsClient = new TestAwContentsClient();
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                // Use detached container view to avoid focus request.
+                mTestContainerView = createDetachedAwTestContainerView(mContentsClient);
+                mEditText = new EditText(getActivity());
+                getActivity().addView(mEditText);
+                getActivity().addView(mTestContainerView);
+            }
+        });
+
+        final CallbackHelper loadHelper = mContentsClient.getOnPageFinishedHelper();
+
+        final String mime = "text/html";
+        final String htmlDocument = "<html><body contenteditable id='editor'></body></html>";
+
+        loadDataSync(mTestContainerView.getAwContents(), loadHelper, htmlDocument, mime, false);
+    }
+
+    private void focusOnEditTextAndShowKeyboard() {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mEditText.requestFocus();
+                InputMethodManager imm = (InputMethodManager) getActivity().getSystemService(
+                        Context.INPUT_METHOD_SERVICE);
+                imm.showSoftInput(mEditText, 0);
+            }
+        });
+    }
+
+    private void focusOnWebViewAndEnableEditing() throws Exception {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mTestContainerView.requestFocus();
+            }
+        });
+
+        enableJavaScriptOnUiThread(mTestContainerView.getAwContents());
+        // This is the usual pattern for email client using webview:
+        // we want the cursor to be there even without tapping the editor.
+        executeJavaScriptAndWaitForResult(mTestContainerView.getAwContents(), mContentsClient,
+                "(function() {"
+                + "var sel = window.getSelection();"
+                + "var range = document.createRange();"
+                + "var editor = document.getElementById('editor');"
+                + "range.setStart(editor, 0);"
+                + "range.setEnd(editor, 0);"
+                + "range.collapse(false);"
+                + "sel.removeAllRanges();"
+                + "sel.addRange(range);"
+                + "})();");
+    }
+
+    private void waitForNonNullInputConnection() throws InterruptedException {
+        CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                InputConnection inputConnection = mTestContainerView.getContentViewCore()
+                        .getImeAdapterForTest().getInputConnectionForTest();
+                return inputConnection != null;
+            }
+        });
+    }
+
+    // https://crbug.com/569556
+    @SmallTest
+    @Feature({"AndroidWebView", "TextInput"})
+    public void testPressNextFromEditTextAndType() throws Throwable {
+        focusOnEditTextAndShowKeyboard();
+        focusOnWebViewAndEnableEditing();
+        waitForNonNullInputConnection();
+    }
+}
\ No newline at end of file
--- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
@@ -142,6 +142,7 @@ public class ImeAdapter {
        // ImeAdapter#dispatchKeyEvent().
        if (mTextInputType == TextInputType.NONE) {
            mInputConnection = null;
+            Log.d(TAG, "onCreateInputConnection returns null.");
            return null;
        }

@@ -155,6 +156,7 @@ public class ImeAdapter {
        int initialSelEnd = outAttrs.initialSelEnd = Selection.getSelectionEnd(mEditable);
        mInputConnection = mInputConnectionFactory.get(
                mViewEmbedder.getAttachedView(), this, initialSelStart, initialSelEnd, outAttrs);
+        Log.d(TAG, "onCreateInputConnection");
        return mInputConnection;
    }

@@ -242,12 +244,6 @@ public class ImeAdapter {
            int textInputFlags, boolean showIfNeeded) {
        Log.d(TAG, "updateKeyboardVisibility: type [%d->%d], flags [%d], show [%b], ",
                mTextInputType, textInputType, textInputFlags, showIfNeeded);
-        // If current input type is none and showIfNeeded is false, IME should not be shown
-        // and input type should remain as none.
-        if (mTextInputType == TextInputType.NONE && !showIfNeeded) {
-            return;
-        }
-
        mTextInputFlags = textInputFlags;
        if (mTextInputType != textInputType) {
            mTextInputType = textInputType;

--- a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
+++ b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
@@ -978,9 +978,11 @@ public class ImeTest extends ContentShellTestBase {
        CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
            @Override
            public boolean isSatisfied() {
-                boolean hasConnection = getAdapterInputConnection() != null;
-                return show == mInputMethodManagerWrapper.isShowWithoutHideOutstanding()
-                        && show == hasConnection;
+                // We do not check the other way around: in some cases we need to keep
+                // input connection even when the last known status is 'hidden'.
+                // See crbug.com/569332 for more details.
+                if (show && getAdapterInputConnection() == null) return false;
+                return show == mInputMethodManagerWrapper.isShowWithoutHideOutstanding();
            }
        });
    }