Support InputMethodManager#updateCursorAnchorInfo for Android 5.0

RESOLVED FIXED in Firefox 50

Status

()

Firefox for Android
Keyboards and IME
--
enhancement
RESOLVED FIXED
3 years ago
11 months ago

People

(Reporter: Yohei Yukawa, Assigned: m_kato)

Tracking

({inputmethod})

Trunk
Firefox 50
All
Android
inputmethod
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8518068 [details]
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
(Assignee)

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

3 years ago
Keywords: inputmethod
(Assignee)

Updated

2 years ago
Depends on: 1141453
(Assignee)

Updated

a year ago
Depends on: 1203871
(Reporter)

Comment 1

a year ago
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
(Assignee)

Updated

a year ago
Assignee: nobody → m_kato
(Assignee)

Comment 2

a year ago
Created attachment 8767108 [details] [diff] [review]
Part 1. Add utility function to convert  Gecko's CSS Rect to LayerView's Rect
(Assignee)

Comment 3

a year ago
Created attachment 8767109 [details] [diff] [review]
Part 2. Support floating candidate window  using hardware keyboard on Android
(Assignee)

Updated

a year ago
Attachment #8767108 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8767109 - Attachment is obsolete: true
(Assignee)

Comment 4

a year ago
Created attachment 8767552 [details]
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/
Attachment #8767552 - Flags: review?(nchen)
Attachment #8767553 - Flags: review?(nchen)
(Assignee)

Comment 5

a year ago
Created attachment 8767553 [details]
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/
Attachment #8767553 - Flags: review?(nchen)
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();
    }
Attachment #8767552 - Flags: review?(nchen) → review?(snorp)
Attachment #8767552 - Flags: review?(snorp) → review+
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
(Assignee)

Comment 8

a year ago
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/
Attachment #8767552 - Attachment description: Bug 1094729 - Part 1. Add utility function to convert Gecko coords to Layer view coords. → Bug 1094729 - Part 1. Add utility function to convert Gecko's coords to Layer view coords.
Attachment #8767553 - Flags: review?(nchen)
(Assignee)

Comment 9

a year ago
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/
Attachment #8767553 - Flags: review?(nchen)
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
(Assignee)

Comment 11

11 months ago
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/
Attachment #8767553 - Flags: review?(nchen)
(Assignee)

Comment 12

11 months ago
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 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!
Attachment #8767553 - Flags: review?(nchen) → review+

Comment 14

11 months ago
Pushed by m_kato@ga2.so-net.ne.jp:
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

11 months ago
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c9d37d6c9ff
Fix bustage. r=me

Comment 16

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9ee0cce1cbe5
https://hg.mozilla.org/mozilla-central/rev/30f12e33d4ea
https://hg.mozilla.org/mozilla-central/rev/5c9d37d6c9ff
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.