Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions fml/platform/android/jni_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,21 @@ std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) {
return JavaStringToString(env, exception_string.obj());
}

void ThrowException(JNIEnv* env, const char* class_name, const char* message) {
jclass clazz = env->FindClass(class_name);
FML_DCHECK(clazz);
env->ThrowNew(clazz, message);
}

ScopedJavaStringChars::ScopedJavaStringChars(JNIEnv* env, jstring str)
: env_(env), str_(str) {
chars_ = env_->GetStringChars(str_, nullptr);
FML_DCHECK(chars_);
}

ScopedJavaStringChars::~ScopedJavaStringChars() {
env_->ReleaseStringChars(str_, chars_);
}

} // namespace jni
} // namespace fml
15 changes: 15 additions & 0 deletions fml/platform/android/jni_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ bool ClearException(JNIEnv* env);

std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable);

void ThrowException(JNIEnv* env, const char* class_name, const char* message);

class ScopedJavaStringChars {
public:
ScopedJavaStringChars(JNIEnv* env, jstring str);
~ScopedJavaStringChars();

const jchar* chars() { return chars_; }

private:
JNIEnv* env_;
jstring str_;
const jchar* chars_;
};

} // namespace jni
} // namespace fml

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,15 @@ public static native void nativeOnVsync(
@NonNull
public static native FlutterCallbackInformation nativeLookupCallbackInformation(long handle);

/**
* Return the index of the first grapheme cluster preceding {@code offset} in {@code text}.
*
* <p>This is similar to {@link android.text.TextUtils#getOffsetBefore(CharSequence, int, int)}
* but it uses the engine's ICU library which is up to date and acts consistently on all versions
* of Android.
*/
public static native int nativeGetTextOffsetBefore(String text, int offset);

@Nullable private Long nativePlatformViewId;
@Nullable private AccessibilityDelegate accessibilityDelegate;
@Nullable private PlatformMessageHandler platformMessageHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@
import android.text.Layout;
import android.text.Selection;
import android.text.TextPaint;
import android.text.method.TextKeyListener;
import android.view.KeyEvent;
import android.view.View;
import android.view.inputmethod.BaseInputConnection;
import android.view.inputmethod.EditorInfo;
import android.view.inputmethod.ExtractedText;
import android.view.inputmethod.ExtractedTextRequest;
import android.view.inputmethod.InputMethodManager;
import androidx.annotation.VisibleForTesting;
import io.flutter.Log;
import io.flutter.embedding.engine.FlutterJNI;
import io.flutter.embedding.engine.systemchannels.TextInputChannel;

class InputConnectionAdaptor extends BaseInputConnection {
Expand Down Expand Up @@ -111,6 +112,23 @@ public InputConnectionAdaptor(
mImm = (InputMethodManager) view.getContext().getSystemService(Context.INPUT_METHOD_SERVICE);
}

interface GetOffsetBefore {
int getOffsetBefore(String text, int offset);
}

private GetOffsetBefore getOffsetBefore =
new GetOffsetBefore() {
@Override
public int getOffsetBefore(String text, int offset) {
return FlutterJNI.nativeGetTextOffsetBefore(text, offset);
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Don't we have access to lambdas?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK the Android embedding does not use lambdas. The embedding supports Android versions back to API 16 and tries to avoid anything that would make it more difficult to support legacy toolchains and runtimes.


@VisibleForTesting
void setGetOffsetBefore(GetOffsetBefore value) {
getOffsetBefore = value;
}

// Send the current state of the editable to Flutter.
private void updateEditingState() {
// If the IME is in the middle of a batch edit, then wait until it completes.
Expand Down Expand Up @@ -270,18 +288,16 @@ public boolean sendKeyEvent(KeyEvent event) {
if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
int selStart = clampIndexToEditable(Selection.getSelectionStart(mEditable), mEditable);
int selEnd = clampIndexToEditable(Selection.getSelectionEnd(mEditable), mEditable);
if (selStart == selEnd && selStart > 0) {
// Extend selection to the left of the last character.
selStart = getOffsetBefore.getOffsetBefore(mEditable.toString(), selStart);
}
if (selEnd > selStart) {
// Delete the selection.
Selection.setSelection(mEditable, selStart);
mEditable.delete(selStart, selEnd);
updateEditingState();
return true;
} else if (selStart > 0) {
if (TextKeyListener.getInstance().onKeyDown(null, mEditable, event.getKeyCode(), event)) {
updateEditingState();
return true;
}
return false;
}
} else if (event.getKeyCode() == KeyEvent.KEYCODE_DPAD_LEFT) {
int selStart = Selection.getSelectionStart(mEditable);
Expand Down
29 changes: 29 additions & 0 deletions shell/platform/android/platform_view_android_jni.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <android/native_window_jni.h>

#include <utility>
#include "unicode/brkiter.h"

#include "flutter/assets/directory_asset_bundle.h"
#include "flutter/common/settings.h"
Expand Down Expand Up @@ -484,6 +485,28 @@ static void InvokePlatformMessageEmptyResponseCallback(JNIEnv* env,
);
}

// Return the index of the first grapheme cluster that precedes the given
// offset in the text.
static jint GetTextOffsetBefore(JNIEnv* env,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a tiny docstring for this function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

jclass jcaller,
jstring text,
jint offset) {
std::unique_ptr<icu::BreakIterator> breaker;
UErrorCode status = U_ZERO_ERROR;
breaker.reset(
icu::BreakIterator::createCharacterInstance(icu::Locale(), status));
if (!U_SUCCESS(status)) {
fml::jni::ThrowException(env, "java/lang/IllegalStateException",
"unable to create character iterator");
return -1;
}

fml::jni::ScopedJavaStringChars chars(env, text);
breaker->setText(
icu::UnicodeString(false, chars.chars(), env->GetStringLength(text)));
return breaker->preceding(offset);
}

bool RegisterApi(JNIEnv* env) {
static const JNINativeMethod flutter_jni_methods[] = {
// Start of methods from FlutterJNI
Expand Down Expand Up @@ -599,6 +622,12 @@ bool RegisterApi(JNIEnv* env) {
.signature = "(J)Lio/flutter/view/FlutterCallbackInformation;",
.fnPtr = reinterpret_cast<void*>(&LookupCallbackInformation),
},

{
.name = "nativeGetTextOffsetBefore",
.signature = "(Ljava/lang/String;I)I",
.fnPtr = reinterpret_cast<void*>(&GetTextOffsetBefore),
},
};

if (env->RegisterNatives(g_flutter_jni_class->obj(), flutter_jni_methods,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import android.text.InputType;
import android.text.Selection;
import android.text.SpannableStringBuilder;
import android.text.TextUtils;
import android.view.KeyEvent;
import android.view.View;
import android.view.inputmethod.EditorInfo;
Expand Down Expand Up @@ -318,6 +319,13 @@ public void testSendKeyEvent_delKeyDeletesBackward() {
int selStart = 29;
Editable editable = sampleRtlEditable(selStart, selStart);
InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable);
adaptor.setGetOffsetBefore(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to just adjust an old test, where is the test that exercises the new logic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a reimplementation of InputConnectionAdaptor delete key handling that uses a JNI function instead of an Android system API to find the character boundary. Unfortunately the new part is JNI native code, and I don't think the embedding has infrastructure for testing that. The Robolectric tests do not load the engine native code library.

Copy link
Member

@gaaclarke gaaclarke Apr 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked into this in the past, let me try to remember. There are 2 ways you can make unit tests, robolectric or a slower test that runs on an emulator. Robolectric can load JNI but your JNI library has to be compiled for the desktop operating system, which might be easy or near impossible depending on what you are doing. It's near impossible if you are calling native android libraries.

new InputConnectionAdaptor.GetOffsetBefore() {
@Override
public int getOffsetBefore(String text, int offset) {
return TextUtils.getOffsetBefore(text, offset);
}
});

KeyEvent downKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL);

Expand Down