1094729 - Support InputMethodManager#updateCursorAnchorInfo for Android 5.0

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, enhancement)

Description

[Yohei Yukawa]

Attachment: Google Japanese Input 2.16.1968.3 with physical keyboard on native TextView

Android 5.0 SDK introduces a set of new APIs that allow IMEs to be informed of the location where text insertion marker and each character bound in composing text are displayed in the screen.  Like other IME APIs, both the active IME and the focused text input widget need to be aware of those APIs to enable that feature.  The system TextView supports this functionality by default.  Also Google Japanese Input and Google Pinyin start relying on those APIs to support floating candidate window when physical keyboards are attached.  Hence it would be really great if Firefox for Android can support this functionality as well as system TextView.

====
HOW TO TEST THIS FEATURE:
1. Install Android L Preview LPX13D into Nexus 7 [2013] (Wi-Fi) "razor", then install Google Japanese Input 2.16.1968.3 from Google Play.
     http://android-developers.blogspot.com/2014/10/android-50-lollipop-sdk-and-nexus.html
     https://play.google.com/store/apps/details?id=com.google.android.inputmethod.japanese
   Or use Nexus 9, where Android 5.0 and Google Japanese Input are installed by default.
2. Attach a physical keyboard.  Make sure your physical keyboard is recognized by the OS.
3. Make sure Google Japanese Input is chosen as the active IME.
4. Open (NativeApp) Google Search from the home screen, tap the search box to focus, then type something with physical keyboard to make sure that Google Japanese Input shows floating candidate window just below the composition text.  See the attached screenshot.
5. Install Firefox Nightly from the Mozilla website. (Tested with Firefox 36.0a1; 2014-11-05)
6. Open "data:text/html,<textarea>" in Firefox nightly
7. Tap the textarea to focus, then type something with physical keyboard.

EXPECTED:
Google Japanese Input shows the floating candidate window just below the composition text like the native TextView.

ACTUAL:
Google Japanese Input does not show the floating candidate window, as if it was running under Android 4.4 and prior.

====
Basically the event flow looks like as follows.

1. The IME calls InputConnection#requestCursorUpdates to start message flow.
2. The application calls back InputMethodManager#updateCursorAnchorInfo when positional information needs to be updated.
3. The IME receives CursorAnchorInfo object and starts doing whatever it wants.

Although there is no comprehensive document about those new APIs has been available yet, I can provide some random notes about what is the design goal of those APIs and how they are implemented.
- The application can reject the request by returning false from InputConnection#requestCursorUpdates.  Actually this is the default behavior of BaseInputConnection.
- Unlike traditional IME APIs in desktop platforms, those new IME APIs in Android are designed to be push-style (from the application to the IME) rather than pull-style (from the IME to the application).  Hopefully this design would be useful for modern browsers that are heavily relying on asynchronous architecture design.
- The application must provide the coordinate-transformation matrix.  Thus it's OK to use local coordinates for character bounds.  Note that the application must call InputMethodManager#updateCursorAnchorInfo even when only the transformation matrix is changed.

Note that the same feature request to Chromium can be found at http://crbug.com/424866.  It is not fully implemented though.

Also note that system TextView is implementing this feature mostly in this file.
https://android.googlesource.com/platform/frameworks/base/+/lollipop-release/core/java/android/widget/Editor.java

Comment 1

[Yohei Yukawa]

Just FYI, http://crbug.com/424866 is expected to be fixed in Chromium 51.  Here is a side-by-side comparison between Chrome dev 51.0.2681.4 and Firefox Nightly 48.0a1 2016/03/15 on Android N developer preview.

https://drive.google.com/open?id=0ByZK1hhvVhpVN0E2aDlpQUFaZWc

Comment 2

[Makoto Kato [:m_kato] (PTO until 5/6)]

Attachment: Part 1. Add utility function to convert Gecko's CSS Rect to LayerView's Rect

Comment 3

[Makoto Kato [:m_kato] (PTO until 5/6)]

Attachment: Part 2. Support floating candidate window using hardware keyboard on Android

Comment 4

[Makoto Kato [:m_kato] (PTO until 5/6)]

Attachment: Bug 1094729 - Part 1. Add utility function to convert Gecko's coords to Layer view coords.

Review commit: https://reviewboard.mozilla.org/r/62006/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62006/

Comment 5

[Makoto Kato [:m_kato] (PTO until 5/6)]

Attachment: Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

Review commit: https://reviewboard.mozilla.org/r/62008/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62008/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8767553 [details]
Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

https://reviewboard.mozilla.org/r/62008/#review59134

We should only send composition rects in nsWindow.cpp if the IME requests them. So InputConnection should first notify nsWindow of a request, and then nsWindow can start sending composition rects back to InputConnection.

::: mobile/android/base/java/org/mozilla/gecko/GeckoEditableListener.java:42
(Diff revision 1)
>      void notifyIME(int type);
>      void notifyIMEContext(int state, String typeHint, String modeHint, String actionHint);
>      void onSelectionChange(int start, int end);
>      void onTextChange(CharSequence text, int start, int oldEnd, int newEnd);
>      void onDefaultKeyEvent(KeyEvent event);
> +    void onCompositionEventHandled(final String aComposingText, final RectF[] aRects);

Change name to "updateCompositionRects"

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:80
(Diff revision 1)
>      private final ExtractedText mUpdateExtract = new ExtractedText();
>      private boolean mBatchSelectionChanged;
>      private boolean mBatchTextChanged;
>      private final InputConnection mKeyInputConnection;
> +    private CursorAnchorInfo.Builder mCursorAnchorInfoBuilder;
> +    private boolean mCursorNotify;

Rename to "mMonitorCursor"

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:437
(Diff revision 1)
> +            updateCandiateWindow();
> +        }
> +    }
> +
> +    @TargetApi(21)
> +    private void updateCandiateWindow() {

Rename to "updateCursor"

::: widget/android/nsWindow.cpp:2861
(Diff revision 1)
>  
> +static jni::ObjectArray::LocalRef
> +ConvertRectArrayToJavaRectFArray(JNIEnv* aJNIEnv, const nsTArray<LayoutDeviceIntRect>& aRects, const LayoutDeviceIntPoint& aOffset, const CSSToLayoutDeviceScale aScale)
> +{
> +    size_t length = aRects.Length();
> +    jclass objectClass = (jclass)(aJNIEnv->NewGlobalRef(aJNIEnv->FindClass("android/graphics/RectF")));

Use,

    jclass objectClass = sdk::RectF::Context().ClassRef();

That way, you do not need to call DeleteGlobalRef afterwards.

::: widget/android/nsWindow.cpp:2878
(Diff revision 1)
> +    }
> +    return rectsRef;
> +}
> +
> +void
> +nsWindow::GeckoViewSupport::CompositionUpdatedInGecko()

Change name to "nsWindow::GeckoViewSupport::UpdateCompositionRects"

::: widget/android/nsWindow.cpp:2991
(Diff revision 1)
>  
> +        case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED: {
> +            ALOGIME("IME: NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED");
> +
> +            // Hardware keyboard support requires each string rect.
> +            CompositionUpdatedInGecko();

Check API version here,

    if (AndroidBridge::Bridge() && AndroidBridge::Bridge()->GetAPIVersion() >= 21) {
        UpdateCompositionRects();
    }

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8767552 [details]
Bug 1094729 - Part 1. Add utility function to convert Gecko's coords to Layer view coords.

https://reviewboard.mozilla.org/r/62006/#review59230

Comment 8

[Makoto Kato [:m_kato] (PTO until 5/6)]

Comment on attachment 8767552 [details]
Bug 1094729 - Part 1. Add utility function to convert Gecko's coords to Layer view coords.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62006/diff/1-2/

Comment 9

[Makoto Kato [:m_kato] (PTO until 5/6)]

Comment on attachment 8767553 [details]
Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62008/diff/1-2/

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8767553 [details]
Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

https://reviewboard.mozilla.org/r/62008/#review60028

Thank you for the revision! Just a few small changes.

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:428
(Diff revisions 1 - 2)
> +        }
> +        int composingStart = getComposingSpanStart(content);
> +        int composingEnd = getComposingSpanEnd(content);
> +        if (composingStart < 0 || composingEnd < 0) {
> +            if (DEBUG) {
> +                Log.d(LOGTAG, "Already no composition now");

Change to "No composition for updates"

::: mobile/android/base/java/org/mozilla/gecko/GeckoInputConnection.java:468
(Diff revisions 1 - 2)
>          if ((cursorUpdateMode & InputConnection.CURSOR_UPDATE_IMMEDIATE) != 0) {
> -            updateCandiateWindow();
> +            updateCursor();
>          }

setRequestCursorUpdates is asynchronous, so if this is the first time we enable cursor updates, I think updateCursor will fail here, because we have not received any cursor updates yet.

I think we should have three possibilities for setRequestCursorUpdates / OnImeRequestCursorUpdates:

    if ((cursorUpdateMode & InputConnection.CURSOR_UPDATE_IMMEDIATE) != 0) {
        mEditableClient.requestCursorUpdates(ONE_SHOT);
    }

which calls UpdateCompositionRects _once_ in GeckoViewSupport::OnImeRequestCursorUpdates. But it does _not_ affect mIMECursorUpdates.

    if ((cursorUpdateMode & InputConnection.CURSOR_UPDATE_MONITOR) != 0) {
        mEditableClient.requestCursorUpdates(START_MONITOR);
    } else {
        mEditableClient.requestCursorUpdates(STOP_MONITOR);
    }

which sets mIMECursorUpdates to true/false in GeckoViewSupport::OnImeRequestCursorUpdates, and UpdateCompositionRects will be called in GeckoViewSupport::NotifyIME like before.

::: widget/android/nsWindow.cpp:296
(Diff revisions 1 - 2)
>      RefPtr<mozilla::TextRangeArray> mIMERanges;
>      int32_t mIMEMaskEventsCount; // Mask events when > 0
>      bool mIMEUpdatingContext;
>      bool mIMESelectionChanged;
>      bool mIMETextChangedDuringFlush;
> +    bool mIMECursorUpdates;

Rename to mIMEMonitorCursor

Comment 11

[Makoto Kato [:m_kato] (PTO until 5/6)]

Comment on attachment 8767552 [details]
Bug 1094729 - Part 1. Add utility function to convert Gecko's coords to Layer view coords.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62006/diff/2-3/

Comment 12

[Makoto Kato [:m_kato] (PTO until 5/6)]

Comment on attachment 8767553 [details]
Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62008/diff/2-3/

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8767553 [details]
Bug 1094729 - Part 2. Support floating candidate window for hardware keyboard on Android 5+.

https://reviewboard.mozilla.org/r/62008/#review62578

LGTM. Thanks!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ee0cce1cbe5
Part 1. Add utility function to convert Gecko's coords to Layer view coords. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/30f12e33d4ea
Part 2. Support floating candidate window for hardware keyboard on Android 5+. r=jchen

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c9d37d6c9ff
Fix bustage. r=me

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ee0cce1cbe5
https://hg.mozilla.org/mozilla-central/rev/30f12e33d4ea
https://hg.mozilla.org/mozilla-central/rev/5c9d37d6c9ff