Closed NickGerleman closed 1 year ago
One solution we've tried that doesn't involve changing the field type to get rid of suggestive text and avoid the entire problem class is a horrible hacky solution that can work... We made a web view and post a message from the onchange listener to the react view. Maybe that helps find a workaround?
So a native app, with the react runtime, running a webview with a textinput whose sole purpose is to provide state back to the react view. 😅
Hi, this issue also affects Pixel phones based on our logs of ANRs as well as many comments in the other thread. Are you also notifying Google? Or is there another potential issue w/ Pixel phones?
See: https://github.com/facebook/react-native/issues/35155#issuecomment-1298456593 https://github.com/facebook/react-native/issues/35155#issuecomment-1323194564 https://support.google.com/pixelphone/thread/181332682/keyboard-freezing-after-android-13-update?hl=en
Thanks.
@NickGerleman Do we have an ETA on the fix for this? We have received a lot of feedback from our users on this issue, Chat is the core functionality of our app and this issue breaks the core functionality for us unfortunately.
Hi, this issue also affects Pixel phones based on our logs of ANRs as well as many comments in the other thread. Are you also notifying Google? Or is there another potential issue w/ Pixel phones?
See: #35155 (comment) #35155 (comment) https://support.google.com/pixelphone/thread/181332682/keyboard-freezing-after-android-13-update?hl=en
Thanks.
Would you mind creating a separate issue for this? It's hard to tell if the issue is related, but using GBoard on Samsung devices does not repro this specific issue we are tracking.
Hi, this issue also affects Pixel phones based on our logs of ANRs as well as many comments in the other thread. Are you also notifying Google? Or is there another potential issue w/ Pixel phones? See: #35155 (comment) #35155 (comment) https://support.google.com/pixelphone/thread/181332682/keyboard-freezing-after-android-13-update?hl=en Thanks.
Would you mind creating a separate issue for this? It's hard to tell if the issue is related, but using GBoard on Samsung devices does not repro the issue.
App still becomes unresponsive when using GBoard on Samsung with Android 13. I just verified that.
our application is running fine on $100 Android Devices but freezes and crashes on expensive Samsung Devices!
One solution we've tried that doesn't involve changing the field type to get rid of suggestive text and avoid the entire problem class is a horrible hacky solution that can work... We made a web view and post a message from the onchange listener to the react view. Maybe that helps find a workaround?
So a native app, with the react runtime, running a webview with a textinput whose sole purpose is to provide state back to the react view. 😅
we had tried this but we ask our clients to disable grammerly to avoid any issue :)
This issue is also affecting older Android and One UI versions. We mitigated this by disabling autoCorrect and spellCheck but it exploded on Android 13.
For two years our UX was(is) terrible. In the recent discussion thread about the future of react-native I had pointed the issue while linking all open issues by then but it fell through the cracks and nobody(?) noticed it until one week ago. Don’t know if I should feel disappointed or frustrated or nothing about it since rn is open source and we all love you guys and gals for your work.
our application is running fine on $100 Android Devices but freezes and crashes on expensive Samsung Devices!
in our app too.
Hey folks! We think we have a mitigation for common cases of this issue. Please see PR https://github.com/facebook/react-native/pull/35967 for some more information about where it may or may not work.
For folks impacted by hangs specific to the Samsung keyboard on Android 13, we would like feedback on whether the change resolves the issue. Based on the signal we get, we will backport to RN 0.68+. Please give feedback directly on the PR.
Please note that because this package impacts build artifacts, tools like patch-package
or local edits to the source will not result in a change if consuming React Native by NPM package. It can instead be tested either by building React Native from source, or by updating to a nightly we will publish soon after the change is merged (it looks like it will just miss the one scheduled to start 20 minutes from now). I will update this comment once a nightly is available with the fix, later today or tomorrow.
Edit: React Native 0.0.0-20230126-2108-53932d002 has this fix.
@NickGerleman thank you for the update! Really appreciate it.
Not sure if there are nightlies being built for 0.68/0.69, but if the fix is backported earlier to those, we'll be happy to help with testing. Unfortunately can't update quickly to 0.71 right now.
We're currently looking to see if we can build from sources to test the fix, but with the backport, we would be able to help test faster!
@NickGerleman I tested by replicating the changes mentioned in the PR in our repo's node module files. I still see the hangs.
What worked for us: Our app uses FlatList + TextInput and in one of the related issues it was mentioned that inverted FlatList worsens the performance. Removing the inverted prop drops the hangs to almost 0%.For anyone with a similar setup, I can confirm that removing the inverted prop definitely fixes the hang/unresponsive issue.
As a reminder, we're only looking for feedback in this particular thread from folks impacting by hangs specific to Samsung keyboard on A13, and that applying the fix to node_modules will not apply the change since it is reliant on packaged build artifacts.
@NickGerleman thank you for the update! Really appreciate it.
Not sure if there are nightlies being built for 0.68/0.69, but if the fix is backported earlier to those, we'll be happy to help with testing. Unfortunately can't update quickly to 0.71 right now.
We're currently looking to see if we can build from sources to test the fix, but with the backport, we would be able to help test faster!
@dozoisch I pushed a new revision, which should work on Fabric, and also reduce the number of Spans in more complicated cases like non-uniform text sizes. We may end up just backporting this into the wild. I don't know we will get a lot of folks who will use the nightly or build from source.
Thanks, @NickGerleman! I saw that it landed on main yesterday. Happy to test it as soon as it gets published on 0.68 or 0.69. Keep us posted 🙌
Hi all,
As @NickGerleman mentioned, we released a nightly version (specifically 0.0.0-20230126-2108-53932d002
) that can be used to verify that the issue is solved. We'd be happy if someone could try it out and confirm that the fix is effectively working.
Just a heads up that nightlies are considered not production ready as they haven't been tested as thoroughly as stable releases. This is just a mechanism we have in place to let developers evaluate a feature or test a patch like in this case.
npx react-native init RNNightly --version nightly
Creating a new project is going to be the easiest way to test this.
Testing this patch in your existing project can be done as follows:
0.71.1
diff --color -r 0.71.1/RNDiffProj/.eslintrc.js nightly/RNDiffProj/.eslintrc.js
3c3
< extends: '@react-native-community',
---
> extends: '@react-native',
diff --color -r 0.71.1/RNDiffProj/.gitignore nightly/RNDiffProj/.gitignore
63a64,66
>
> # testing
> /coverage
diff --color -r 0.71.1/RNDiffProj/.ruby-version nightly/RNDiffProj/.ruby-version
1c1
< 2.7.6
---
> 2.7.0
diff --color -r 0.71.1/RNDiffProj/.watchmanconfig nightly/RNDiffProj/.watchmanconfig
1c1
< {}
\ No newline at end of file
---
> {}
Only in 0.71.1/RNDiffProj/__tests__: App-test.tsx
Only in nightly/RNDiffProj/__tests__: App.test.tsx
diff --color -r 0.71.1/RNDiffProj/_node-version nightly/RNDiffProj/_node-version
1c1
< 16
---
> 18
diff --color -r 0.71.1/RNDiffProj/android/app/build.gradle nightly/RNDiffProj/android/app/build.gradle
16,17c16,17
< // The folder where the react-native Codegen package is. Default is ../node_modules/react-native-codegen
< // codegenDir = file("../node_modules/react-native-codegen")
---
> // The folder where the react-native Codegen package is. Default is ../node_modules/@react-native/codegen
> // codegenDir = file("../node_modules/@react-native/codegen")
diff --color -r 0.71.1/RNDiffProj/android/app/src/main/res/drawable/rn_edit_text_material.xml nightly/RNDiffProj/android/app/src/main/res/drawable/rn_edit_text_material.xml
23c23
< <!--
---
> <!--
Binary files 0.71.1/RNDiffProj/android/gradle/wrapper/gradle-wrapper.jar and nightly/RNDiffProj/android/gradle/wrapper/gradle-wrapper.jar differ
diff --color -r 0.71.1/RNDiffProj/android/gradle/wrapper/gradle-wrapper.properties nightly/RNDiffProj/android/gradle/wrapper/gradle-wrapper.properties
3c3
< distributionUrl=https\://services.gradle.org/distributions/gradle-7.5.1-all.zip
---
> distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
diff --color -r 0.71.1/RNDiffProj/android/gradle.properties nightly/RNDiffProj/android/gradle.properties
28c28
< FLIPPER_VERSION=0.125.0
---
> FLIPPER_VERSION=0.174.0
diff --color -r 0.71.1/RNDiffProj/android/gradlew nightly/RNDiffProj/android/gradlew
207a208,213
> # Stop when "xargs" is not available.
> if ! command -v xargs >/dev/null 2>&1
> then
> die "xargs is not available"
> fi
>
diff --color -r 0.71.1/RNDiffProj/android/gradlew.bat nightly/RNDiffProj/android/gradlew.bat
17c17
< @if "%DEBUG%" == "" @echo off
---
> @if "%DEBUG%"=="" @echo off
28c28
< if "%DIRNAME%" == "" set DIRNAME=.
---
> if "%DIRNAME%"=="" set DIRNAME=.
43c43
< if "%ERRORLEVEL%" == "0" goto execute
---
> if %ERRORLEVEL% equ 0 goto execute
78c78
< if "%ERRORLEVEL%"=="0" goto mainEnd
---
> if %ERRORLEVEL% equ 0 goto mainEnd
83,84c83,86
< if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
< exit /b 1
---
> set EXIT_CODE=%ERRORLEVEL%
> if %EXIT_CODE% equ 0 set EXIT_CODE=1
> if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE%
> exit /b %EXIT_CODE%
diff --color -r 0.71.1/RNDiffProj/android/settings.gradle nightly/RNDiffProj/android/settings.gradle
4c4
< includeBuild('../node_modules/react-native-gradle-plugin')
---
> includeBuild('../node_modules/@react-native/gradle-plugin')
diff --color -r 0.71.1/RNDiffProj/app.json nightly/RNDiffProj/app.json
4c4
< }
\ No newline at end of file
---
> }
diff --color -r 0.71.1/RNDiffProj/ios/Podfile nightly/RNDiffProj/ios/Podfile
33,34d32
< # Upcoming versions of React Native may rely on get_default_flags(), but
< # we make it explicit here to aid in the React Native upgrade process.
51a50
> # https://github.com/facebook/react-native/blob/main/scripts/react_native_pods.rb#L197-L202
54,55d52
< # Set `mac_catalyst_enabled` to `true` in order to apply patches
< # necessary for Mac Catalyst builds
diff --color -r 0.71.1/RNDiffProj/ios/RNDiffProj/AppDelegate.mm nightly/RNDiffProj/ios/RNDiffProj/AppDelegate.mm
26c26
< /// This method controls whether the `concurrentRoot`feature of React18 is turned on or off.
---
> /// This method controls whether the `concurrentRoot` feature of React18 is turned on or off.
Only in nightly/RNDiffProj: jest.config.js
diff --color -r 0.71.1/RNDiffProj/package.json nightly/RNDiffProj/package.json
14c14
< "react-native": "0.71.1"
---
> "react-native": "0.0.0-20230126-2108-53932d002"
19,20c19,20
< "@babel/runtime": "^7.20.0",
< "@react-native-community/eslint-config": "^3.0.0",
---
> "@babel/runtime": "^7.12.5",
> "@react-native/eslint-config": "^0.72.1",
28c28
< "metro-react-native-babel-preset": "0.73.7",
---
> "metro-react-native-babel-preset": "0.73.5",
32,34d31
< },
< "jest": {
< "preset": "react-native"
As nightlies are not supposed to be used for production, once we have enough confidence that this fix is working well, we'll be pushing point releases for our stable versions.
For the time being, please comment to this issue by providing feedback on the latest nightly and we'll keep you posted on how we're going to proceed.
Applying the patch from the PR and building from source did not solve our ANRs on Samsung devices running Android 13.
@rikur could you help by clarifying:
FYI, I have created a new Nightly project and tested with the Samsung Remote Test Lab, using S22 with Android 13. I can no longer reproduce the lag / ANR with the built-in Grammarly plugin turned on.
@rikur could you help by clarifying:
1. How you built the patched app from source?
We've extracted a recent version of TextInput from react-native into a package, so that we can build it from source without having to build all of react-native from source. I applied the patch to it. I admit that there's a chance that this way of building TextInput might break the patch.. but all other customizations thus far have worked great 😅
- Is your TextInput controlled or uncontrolled? uncontrolled.
- Do you use any formatting in your TextInput? We use a custom font, but not "rich text" formatting.
- Does your scenario also lag with GBoard? No.
Thank you! We have applied this patch to RN 0.70.6 and built it from the source. And now the ANR is gone.
@rikur thanks for the feedback!
I'm going to take a closer look at the scenarios for font size setting layout impacting spans. To debug this further, it could be helpful to have a couple of things:
mSpans
field on the string. This will also confirm whether or not code from the patch is running.hey folks, we have started rolling out the mitigation Nick and Nicola worked on for 0.68 and 0.69 so that you can try it out more easily in your codebases:
0.70 and 0.71 will be done later in the week, there's more work to do on those branches.
please try them out and report if they help addressing the issue, your feedback is fundamental for our understanding of the problem 🙏
@kelset thank you! preparing a build internally :)
@kelset Are the changes out for 0.69.8 for all users or are they still rolling out?
Will there be a patch for 0.67.x as well?
I tested on 0.69.8 with (Inverted Flatlist + Text Input) on A 13 device and i am still seeing the lags + ANR.
To set expectations, this patch will not address issues due to inverted FlatList, or likely any issues which were causing hangs in more than just the Samsung case. We are aware of the separate issue with inverted FlatList though, but have not started any investigation on it.
Will there be a patch for 0.67.x as well?
no, we will not go below 0.68.
I tested the nightly, and I could not reproduce the issue. I believe the issue was fixed by Nick's solution. I installed the example from https://github.com/facebook/react-native/issues/35590 on a Samsung running Android 13 + Grammarly Keyboard integration. I remain available for further testing tomorrow other scenarios. Thanks.
@NickGerleman @kelset ran the patch on two devices and ran both 0.68.6 and 0.69.8 and unfortunately
We use controlled input, using Roboto
I got some stack traces for the S22.
@dozoisch from the stacktrace on the S22 I am seeing an issue where something listening to the TextInput for autofill is crashing when it itself is trying to persist too much text. Are you using any third-party tools which may be listening for this?
@NickGerleman as far as I know we aren’t.. and the weird part was that the input was barely 4-5 sentences long
From the callstack I suspect we are seeing a different issue, but if you are able to break on unhandled exception, it may be useful to see the text (including spans) being committed to the EditText which is triggering the behavior.
I’m testing from Samsung test lab so can’t put a breakpoint unfortunately, but I’ll try to see if I can get a video of it, and then also see if we get any breakpoint from the s10 device
Can confirm 0.68.6 fixes the issue for me, I'm also using a patch to fix the invterted flatlist issues on android and the combination of the two fixes completely removes the lag for me, thanks guys!!
Can confirm 0.68.6
fixes it for us too 🙌 Thanks!
patch to fix the invterted flatlist issues
@moulie415 is that patch a PR already open against code by any chance? Could you set it up as a PR if not? I fear that folks that report still issues with the latest patches might be caught into the inverted flatlist one (if it's already a patch commented in the dedicated issue, even just a comment link to that would be ok)
@dozoisch your report is somewhat unclear, could you help us understand a bit more which scenario crashes vs unresolved?
Basically, can you copy paste this section and fill accordingly? 👇
S22 android 13 + RN 0.68.6 = ?
S22 android 13 + RN 0.69.8 = ?
S10 android 12 (but with grammarly enabled) + RN 0.68.6 = ?
S10 android 12 (but with grammarly enabled) + RN 0.69.8 = ?
Given how other folks are reporting that for 0.68 it's fixed, I wonder if as @NickGerleman is suggesting, that there's something else going on in your code that might be affecting it. (or maybe there's only an issue with 0.69?)
@kelset yes good point, if there are people like me who are using an inverted flatlist alongside a textinput please apply this patch as well as trying the new releases https://github.com/facebook/react-native/issues/30034#issuecomment-1382869583.
Anyone using react-native-gifted-chat should be wary of this because I think thats the default setup.
@moulie415 stupid question probably, inverted should be true on the list, right?
works as a charm on Samsung S22 Android 13 @ 0.68.6
. I was afraid that it won't work well on an TextInput that handles mentions (lot of sorcery in there). Thank you guys
edit: I've also brought back the loooong disabled autoCorrect
and autoSpell
props, Grammarly is enabled.
repo https://github.com/cortinico/reproducer-grammarly-textinput https://www.icloud.com/iclouddrive/0abUF8gi2tBBSKdlUzGsgxWFw#first_crash_test_with_grammarly
https://www.icloud.com/iclouddrive/056EulKPpwZ-SOT-a88dTvVzQ#nightly_success Log file https://www.icloud.com/iclouddrive/01701zvtygAyH1QJQcyWz1J-w#logs_2023131_112831
@moulie415 stupid question probably, inverted should be true on the list, right?
@efstathiosntonas not sure I quite understand what you mean? It's not true by default on flatlists but if the flatlist is set to true then thats what has been known to cause issues android and thus needs the patch.
@moulie415 that's what I meant, on react-native-gifted-chat
I have inverted
set to true
and I've applied the patch. Thank you.
Tested it on Pixel 6 Pro & Pixel 7 both on Android 13 on browserstack's real devices, no lag. We had some Pixel users complaining about it even though they were using Gboard.
Samsung S21 - Android 13 - 0.68.6: still crashing with "android.os.TransactionTooLargeException: data parcel size 399832 bytes"
same as @dozoisch
Edit: Samsung S22 - Android 13 - 0.68.6 same issue
Both with Grammarly activated
@shawntuckerworkiz can you please post the android build number of the S22?
Will there be a patch for 0.67.x as well?
no, we will not go below 0.68.
Is patch for 0.70 & 0.71 coming soon?
0.70 is getting released at this very moment, 0.71 will come tomorrow-ish
new patch of 0.70 with the fix: https://github.com/facebook/react-native/releases/tag/v0.70.7
NEW UPDATE: we have now released patch releases containing the second wave of fix for:
UPDATE: we have now released a patch release containing the first mitigation for all the supported versions of React Native plus 0.68 (since it's still used by a significant number of users):
Please upgrade to those versions and report back here if it helps! We are still planning to do further investigation, but don't expect any new releases concerning this topic for at least a couple weeks (unless something unexpected happens).
There have been repeated and persistent signals of ANRs (hangs) when typing into a multiline TextInput on Samsung phones. This is most reported by using the out-of-the-box keyboard provided by Samsung One UI 5. We have been able to locally reproduce the hangs on two separate Galaxy S22 devices. Most devices will not be as fast as the ones we have tested on.
Impact
The issue presents as repeated hangs or ANRs when typing into a TextInput which has a large amount of text. It is specific to the Samsung soft keyboard, and does not reproduce when using other keyboards like GBoard on the same device. There are a different set of TextInputs where we have confirmed impact depending on whether Fabric is enabled or disabled.
Fabric enabled
Both controlled and uncontrolled TextInputs are impacted.
Fabric disabled
Controlled TextInputs and uncontrolled TextInputs which set an initial value via child are impacted.
Root causing
Root causing the issue focused on:
Profiling
We saw similar results to the trace already shared in #35590. A Java sampling profiler shows the process
com.samsung.android.honeyboard
has a very long running frame, delivering input.This triggers code on the main thread, in the app process, where we see a hang happen while committing a text change from the IME to the underlying Spannable backing the EditText. The hang happens in an event handler which has subscribed to changes to the underlying text. The long-running work seems to be laying out the text.
Because the hang happens in response to an edit to the underlying Spannable, investigation focused on the Spannable content differences in scenarios which created ANRs vs not.
Spannable differences between hang/no hang
We examined typing into the below example, which may hang on Fabric but not Paper:
Grammarly annotations were present as
SuggestionSpan
spans on both platforms. On Fabric but not paper there was the presence ofAbsoluteSizeSpan
spans, which set the font size within the text. This is akin to setting<span style=”font-size: 12px”}>Text</span>
within a browser.When we stubbed out platform code in React Native which set an
AbsoluteSizeSpan
, that example along with another previously hanging example would no longer hang. The hang was possible to trigger in an uncontrolled TextInput with a short string like “Text Here” as a child, so we attempted to repro the logical extreme, where we add a single span setting font size to the TextInput.Reproducing outside of React Native
We discovered that adding even a single inclusive span (specifically setting font size) is enough to create hangs. This can reproduced by:
EditText
with aninputType
oftextMultiLine
.str.setSpan(new AbsoluteSizeSpan(55), 0, string.length(), Spanned.SPAN_INCLUSIVE_INCLUSIVE)
We were able to create a repro for this on top of a template Android application. The issue only happens when using the Samsung keyboard.
https://user-images.githubusercontent.com/835219/214149647-7dd093de-29d0-4a62-b898-579316ee1365.mp4
Grammarly annotations are not shown locally when changing the appId and namespace of the repro toThis only happened on 1/2 devices, so it's possible there might be some setting or association that wasn't fresh.com.facebook.katana
(the ID of the Facebook app). This implies the Facebook app may be being special-cased, contributing to the lack of internal signal on the issue.Remediation
While the underlying issue seems to be outside of React Native, the issue still has a real-world effect on our users. We are starting the process to mitigate the issue by working to notify Samsung. This issue has already shown signs of impacting the Android app ecosystem outside of React Native and the ability to set spans with formatting like font size is a fundamentally important part of the platform.
We hope this issue will be addressed in a future update from Samsung, but this does not address the already-present impact of the issue. A potential mitigation which we are exploring is whether React Native could avoid adding AbsoluteSizeSpan (or formatting spans in general) in more common cases.
There have been previously discovered workarounds for the issue, such as setting
keyboardType
tovisible-password
. We recommend against preemptively using these workarounds, due to impact on UX and accessibility.