Fix - TextInput Drawable to avoid Null Pointer Exception RuntimeError #17530

[fabOnReact]

Summary

This issue fixes #17530 fixes expo/expo#9905 with the help of @sunnylqm https://github.com/sunnylqm

Re-rendering a large number of TextInputs with key prop on the screen will trigger the below Null Pointer Exception Runtime Error

NullPointerException:tempt to invoke virtual method 'android.graphics.drawable.Drawable android.graphics.drawable.Drawable$ConstantState.newDrawable(android.content.res.Resources)'

The error is caused by null.newDrawable(mSourceRes) at

https://github.com/aosp-mirror/platform_frameworks_base/blob/20b012282e0c3d94b5c0aa799cdda065f2df06db/graphics/java/android/graphics/drawable/DrawableContainer.java#L919

More info #29452 (comment) #17530 (comment)

The Theme Theme.AppCompat.Light.NoActionBar defines the Drawables for AppCompatEditText in @drawable/abc_edit_text_material.xml

https://chromium.googlesource.com/android_tools/+/7200281446186c7192cb02f54dc2b38e02d705e5/sdk/extras/android/support/v7/appcompat/res/drawable/abc_edit_text_material.xml

Removing the following line from the above xml file @drawable/abc_edit_text_material.xml fixes the error #17530 (comment)

<item android:state_pressed="false" android:state_focused="false" android:drawable="@drawable/abc_textfield_default_mtrl_alpha"/>

The Theme default EditText background is replaced with a custom background, which is a copy of the original background without the above item which triggers the Runtime Error. The changes are implemented in RNTester with commit (more info in the commit) 0858d41. The new custom drawable used as default background for the TextInput is named edit_text.

<item name="android:editTextBackground">@drawable/edit_text</item>

The same changes have been added to react-native default template for creating new applications with commit (more info) f349308, lean core moved the cli tools to https://github.com/react-native-community/cli, but the default template for creating a new application is stored in facebook/react-native/template.

New applications will be generated with this configurations and will not experience the error, existing react-native applications will fix the error by upgrading with the upgrade-helper.

A Minimum Reproducible Example to reproduce this error is included in commit (more info in the commit) fabOnReact@4a414e2 and #17530 (comment)

Changelog

[Android] [Fixed] - TextInput Drawable to avoid Null Pointer Exception RuntimeError #17530

Test Plan

Works in all scenarios on Android.

CLICK TO OPEN TESTS RESULTS - React Native

Test Results from the Testing in RNTester.

Minimum Reproducible Example added with commit (more info in the commit) fabOnReact@4a414e2

The example included in commit fabOnReact@4a414e2 will cause a NPE Runtime Error on Master Branch, while no error is experienced in the feature branch. The links are video hosted on s3 of this tests (playable by google chrome).

BEFORE	AFTER

The below screenshots were taken to detect any issues with the EditText Background. There is no difference between master and feature branch.

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

BEFORE	AFTER

AFTER

CLICK TO OPEN TESTS RESULTS - React Native Cli

As lean core move cli tools to https://github.com/react-native-community/cli, I tested the changes to the template in a separate repository https://github.com/fabriziobertoglio1987/react-native-template and generated the template with the following command

npx react-native init ProjectName --template file:///home/fabrizio/Documents/sourcecode/opensource/react-native-template/template

The generated app did not experience any issues and includes all the changes in rn_edit_text_material.xml and styles.xml

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,706,586	+645
android	hermes	armeabi-v7a	7,236,706	+638
android	hermes	x86	8,126,630	+636
android	hermes	x86_64	8,091,604	+644
android	jsc	arm64-v8a	9,626,248	+649
android	jsc	armeabi-v7a	8,543,884	+638
android	jsc	x86	9,640,571	+636
android	jsc	x86_64	10,249,229	+646

Base commit: b4ac211

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: b4ac211

[sunnylqm]

Thank you @fabriziobertoglio1987 for your awesome work. I really admire your great patience and endeavor on this haunting issue.

The solution looks good to me, but still the question is not answered throughly: What is the real cause. What is wrong with the drawable?

Why remove <item android:state_pressed="false" android:state_focused="false" android:drawable="@drawable/abc_textfield_default_mtrl_alpha"/> could fix? Why so specific with press/focus state? Remember this exception could also be thrown from a touch event? #18395

And why removing key props could also prevent the exception? Is it because key props will try to reuse the component and hence cached some dirty state?

I know little to dig it like you did, but it keeps me wondering if there is something wrong with react native that managing the ui state.
If the issue lies deep in some managing mechanism, I believe this could not be the only case and I found a very similar one that could be highly related #9979

Anyway, well done!

[fabOnReact]

The Runtime is triggered in DrawableContainer.getChild() because ConstantState variable cs is null. Calling null.newDrawable(mSourceRes) triggers a Null Pointer Exception. This call is the result of the change of state of the TextInput, in this stack trace after pressing it.

The Background abc_edit_text_material.xml used for AppCompatEditText defines StateListDrawable which will change the background image of the TextInput depending on the state.

The lines below are trying to replace the old Background Image with a new one, but the process fails because mDrawableFutures does not include the correct list of entries.

final ConstantState cs = mDrawableFutures.valueAt(keyIndex);
final Drawable prepared = prepareDrawable(cs.newDrawable(mSourceRes));

I am still trying to understand why mDrawableFutures does not include the full list of items. I believe the key to understand this is:

1. Finding where mDrawableFutures is initially set
2. Finding where mDrawableFutures is updated
3. Debug the Android Source

I try to answer your questions with my current knowledge of the problem, sorry if they are not satisfing answers. Tomorrow I will keep working on this and try to find a better solution.

What is the real cause.

The value of mDrawableFutures, how it is initizialized and then updated. I believe this is not the answer you are looking for, but it is an important starting point. Tomorrow I will investigate the role of react-native in setting and updating mDrawableFutures.

What is wrong with the drawable?

I believe removing this Drawable is just a workaround to solve the real issue. The real issue is how mDrawableFutures is set/updated. If this issue is experienced in many different use cases, removing the Drawable may not be a good solution.

Why remove <item android:state_pressed="false" android:state_focused="false" android:drawable="@drawable/abc_textfield_default_mtrl_alpha"/> could fix?
And why removing key props could also prevent the exception? Is it because key props will try to reuse the component and hence cached some dirty state?

Maybe because a TextInput with key props is the only condition to trigger state_pressed=false and state_focused=false, while without a key prop those conditions are not triggered.
Thanks a lot

[sunnylqm]

@fabriziobertoglio1987 I'd like to hire you if I am a boss 👍

Anyway do not let this issue take too much of your time. You do not have to understand it 100% to make it a ready pr and tag some core members to review. If this change prevent the crash without regressions it's already a great valuable contribution to the community.

[fabOnReact]

Thanks a lot @sunnylqm . I don't think that I could have done this Pull Request alone. I believe the interaction was very important to find a solution, the issue was really challenging and I am very happy that we could solve it together.

I wish you a good evening.
Fabrizio

[Skullcan]

@fabriziobertoglio1987 any chance I can apply this fix in an active project w/o waiting for this PR to be merged?

[hieuhlc]

Hi guys, can we merge this PR now?

[nericode]

When will this be ready PR?

[whoami-shubham]

which react-native version would be containing the fix of this issue?

[Uzunoff]

which react-native version would be containing the fix of this issue?

Hello, I believe it will be after 0.63.3 version, because there are people with this version experiencing the same issue.

[sunnylqm]

Steps to land a fix:

1. reviewed and approved by a core member
2. tested and merged to master branch by a core member
3. wait for a core member to cut a new version from master branch or cherry pick this commit to a released version as a patch
4. wait for a core member to release a new version containing this pr

And now we are at stage 0

@Uzunoff @whoami-shubham

[lfoliveir4]

@fabriziobertoglio1987 @lunaleaps Hi! Any predictions for this pull request to be approved?

[facebook-github-bot]

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @fabriziobertoglio1987 in 254493e.

^{When will my fix make it into a release? | Upcoming Releases}

[watsonnnnn]

Is this PR released? I have not found any changelog for it.

[MaeIg]

Is this PR released? I have not found any changelog for it.

Hello,

You can check here if this fix is planned for a future release.

It should be in v0.67 !

[lennartkloock]

Is this already merged? I'm using version 0.68.2 and this still happens exactly as described.

[rafaelnco]

hey guys i have been able to cut off this crash on JS side, do you think it is reliable? i am on RN 0.62