Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fixing bug when ListenableEditingState is provided initialState with a valid composing range, android.view.inputmethod.BaseInputConnection.setComposingRegion(int, int) has NPE #30916

Merged
fluttergithubbot merged 4 commits into from
Jan 20, 2022
Merged

Fixing bug when ListenableEditingState is provided initialState with a valid composing range, android.view.inputmethod.BaseInputConnection.setComposingRegion(int, int) has NPE #30916

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38d6cff
Update ListenableEditingState.java
fzyzcjy Jan 19, 2022
0a6fdb6
Update ListenableEditingStateTest.java
fzyzcjy Jan 19, 2022
ca21493
Update ListenableEditingStateTest.java
fzyzcjy Jan 19, 2022
b076bb0
Update ListenableEditingState.java
fzyzcjy Jan 19, 2022
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
6 changes: 5 additions & 1 deletion shell/platform/android/io/flutter/plugin/editing/ListenableEditingState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ public void setComposingRange(int composingStart, int composingEnd) {
if (composingStart < 0 || composingStart >= composingEnd) {
BaseInputConnection.removeComposingSpans(this);
} else {
mDummyConnection.setComposingRegion(composingStart, composingEnd);
if (mDummyConnection != null) {
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Jan 19, 2022

Choose a reason for hiding this comment

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

Does it make more sense to move the mDummyConnection initialization to before the setEditingState call in the constructor?

Copy link
Contributor Author

@fzyzcjy fzyzcjy Jan 19, 2022

Choose a reason for hiding this comment

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

I am not sure. Notice mDummyConnection is init as:

    Editable self = this;
    mDummyConnection =
        new BaseInputConnection(view, true) {
          @Override
          public Editable getEditable() {
            return self;
          }
        };

Where there is a self (indeed this). If we move it at the beginning of constructor, that getEditable can return a not-fully-initialized instance imho

Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Jan 19, 2022

Choose a reason for hiding this comment

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

If I recall correctly the dummy connection is a quirk to get access to the BaseInputConnection.setComposingRegion static method:
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/inputmethod/BaseInputConnection.java;l=104-128;drc=master?q=BaseInputConnection

Since it's called after the super implementation, and all the dummy connection does is changing the current composing region, it should be fine to return the Editable I think (feel free to add a code comment there).

Copy link
Contributor Author

@fzyzcjy fzyzcjy Jan 19, 2022

Choose a reason for hiding this comment

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

fixed

mDummyConnection.setComposingRegion(composingStart, composingEnd);
} else {
Log.w(TAG, "setComposingRange skip since mDummyConnection is null");
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions shell/platform/android/test/io/flutter/plugin/editing/ListenableEditingStateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ public void setUp() {
MockitoAnnotations.openMocks(this);
}

@Test
public void testConstructor() {
// When provided valid composing range, should not fail
new ListenableEditingState(
new TextInputChannel.TextEditState("hello", 1, 4, 1, 4),
new View(RuntimeEnvironment.application));
}

// -------- Start: Test BatchEditing -------
@Test
public void testBatchEditing() {
Expand Down