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

fzyzcjy · 2022-01-19T00:10:48Z

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
See flutter/flutter#96570

List which issues are fixed by this PR. You must list at least one issue.
Fix flutter/flutter#96570

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2022-01-19T00:10:50Z

This pull request was opened against a branch other than main. Since Flutter pull requests should not normally be opened against branches other than main, I have changed the base to main. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than main, unless this is an intentional hotfix/cherrypick.

flutter-dashboard · 2022-01-19T00:10:52Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

flutter-dashboard · 2022-01-19T00:10:52Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

fzyzcjy · 2022-01-19T00:42:51Z

Well cannot understand that CI error... Have searched keyword ListenableEditingState but no related things. The log seems also unrelated to my code change... Could anyone plz give some hints?

Possibly related @chinmaygarde @darshankawar

jason-simmons · 2022-01-19T01:13:49Z

The "Linux Unopt" CI error is asking for a change to the code formatting
(see https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8824653695614065553/+/u/test:_format_and_dart_test/stdout)

The "build_and_test_linux_unopt_debug" CI error is a flaky test that has been seen before (see flutter/flutter#95751)

fzyzcjy · 2022-01-19T01:23:05Z

@jason-simmons thanks! done

LongCatIsLooong · 2022-01-19T02:09:27Z

shell/platform/android/io/flutter/plugin/editing/ListenableEditingState.java

@@ -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) {


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

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

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).

LongCatIsLooong

LGTM. Thanks for fixing this!

fzyzcjy · 2022-01-20T00:27:11Z

You are welcome!

…State` with a valid composing range, `android.view.inputmethod.BaseInputConnection.setComposingRegion(int, int)` has NPE (flutter/engine#30916)

kobi666 · 2022-01-21T00:22:43Z

@chinmaygarde
yo,
so when can we expect this fix to reach stable?

is there an HF version?

FabioPaganoAPSystem · 2022-02-03T23:58:28Z

In flutter version "2.10.0-stable" the bug seems not resolved, I've tested with my app.

everskies · 2022-03-28T18:10:32Z

Would be great if this could be merged into stable! This is our most occurring issue at the moment

fzyzcjy · 2022-03-28T23:55:30Z

Given my understanding of flutter, seems that we need to wait until next stable (which is roughly 2 months later, unfortunately) if it is not in 2.10. Or, we may make a cherry-pick request

apoleo88 · 2022-05-24T10:43:33Z

Still get the error with:

Flutter 2.10.4 • channel stable 
Tools • Dart 2.16.2 • DevTools 2.9.2

Specifically when I double-tap to select the text in a text field.

gonzalonm · 2022-08-05T09:49:49Z

Looks like this fix has not been included in stable channel yet. Does anybody know where it was included?

fzyzcjy · 2022-08-05T10:23:34Z

@gonzalonm May I know where you see it not in stable? from the release date it should have been stable

gonzalonm · 2022-08-05T13:49:40Z

I didn't see it in the release notes. Anyways, I could not reproduce the issue with version 3.0.0, so in that version it has been fixed.
Thanks!

fzyzcjy · 2022-08-05T13:51:01Z

It may be in release notes of engine, while flutter/flutter repo (not flutter/engine) will not say this

Update ListenableEditingState.java

38d6cff

flutter-dashboard bot changed the base branch from master to main January 19, 2022 00:10

flutter-dashboard bot added the platform-android label Jan 19, 2022

flutter-dashboard bot added the needs tests label Jan 19, 2022

fzyzcjy mentioned this pull request Jan 19, 2022

When ListenableEditingState is provided initialState with a valid composing range, android.view.inputmethod.BaseInputConnection.setComposingRegion(int, int) has NPE flutter/flutter#96570

Closed

Update ListenableEditingStateTest.java

0a6fdb6

Update ListenableEditingStateTest.java

ca21493

LongCatIsLooong reviewed Jan 19, 2022

View reviewed changes

Update ListenableEditingState.java

b076bb0

LongCatIsLooong approved these changes Jan 19, 2022

View reviewed changes

LongCatIsLooong requested a review from blasten January 19, 2022 19:02

chinmaygarde approved these changes Jan 20, 2022

View reviewed changes

chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jan 20, 2022

fluttergithubbot merged commit d405b2b into flutter:main Jan 20, 2022

engine-flutter-autoroll mentioned this pull request Jan 21, 2022

Roll Engine from 84e48990dc8f to cd65368f9dec (9 revisions) flutter/flutter#96963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

fzyzcjy commented Jan 19, 2022 •

edited

flutter-dashboard bot commented Jan 19, 2022

flutter-dashboard bot commented Jan 19, 2022

flutter-dashboard bot commented Jan 19, 2022

fzyzcjy commented Jan 19, 2022 •

edited

jason-simmons commented Jan 19, 2022

fzyzcjy commented Jan 19, 2022

LongCatIsLooong Jan 19, 2022

fzyzcjy Jan 19, 2022

LongCatIsLooong Jan 19, 2022

fzyzcjy Jan 19, 2022

LongCatIsLooong left a comment

fzyzcjy commented Jan 20, 2022

kobi666 commented Jan 21, 2022

FabioPaganoAPSystem commented Feb 3, 2022

everskies commented Mar 28, 2022 •

edited

fzyzcjy commented Mar 28, 2022

apoleo88 commented May 24, 2022

gonzalonm commented Aug 5, 2022

fzyzcjy commented Aug 5, 2022 •

edited

gonzalonm commented Aug 5, 2022

fzyzcjy commented Aug 5, 2022

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

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

Conversation

fzyzcjy commented Jan 19, 2022 • edited

Pre-launch Checklist

flutter-dashboard bot commented Jan 19, 2022

flutter-dashboard bot commented Jan 19, 2022

flutter-dashboard bot commented Jan 19, 2022

fzyzcjy commented Jan 19, 2022 • edited

jason-simmons commented Jan 19, 2022

fzyzcjy commented Jan 19, 2022

LongCatIsLooong Jan 19, 2022

Choose a reason for hiding this comment

fzyzcjy Jan 19, 2022

Choose a reason for hiding this comment

LongCatIsLooong Jan 19, 2022

Choose a reason for hiding this comment

fzyzcjy Jan 19, 2022

Choose a reason for hiding this comment

LongCatIsLooong left a comment

Choose a reason for hiding this comment

fzyzcjy commented Jan 20, 2022

kobi666 commented Jan 21, 2022

FabioPaganoAPSystem commented Feb 3, 2022

everskies commented Mar 28, 2022 • edited

fzyzcjy commented Mar 28, 2022

apoleo88 commented May 24, 2022

gonzalonm commented Aug 5, 2022

fzyzcjy commented Aug 5, 2022 • edited

gonzalonm commented Aug 5, 2022

fzyzcjy commented Aug 5, 2022

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

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

fzyzcjy commented Jan 19, 2022 •

edited

fzyzcjy commented Jan 19, 2022 •

edited

everskies commented Mar 28, 2022 •

edited

fzyzcjy commented Aug 5, 2022 •

edited