Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigation for Samsung TextInput Hangs #35967

Closed
wants to merge 1 commit into from

Conversation

NickGerleman
Copy link
Contributor

@NickGerleman NickGerleman commented Jan 25, 2023

Summary

In #35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and @cortinico.

Test Plan

For Paper and Fabric, debugged the flow through the algorithm over several examples, along with the effect on underlying TextInput spannable (observed indirectly by text updates).

image

image

image


Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Jan 25, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42721684

@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Jan 25, 2023
@analysis-bot
Copy link

analysis-bot commented Jan 25, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,463,089 +136
android hermes armeabi-v7a 7,783,450 +149
android hermes x86 8,936,072 +146
android hermes x86_64 8,793,971 +134
android jsc arm64-v8a 9,648,917 +23
android jsc armeabi-v7a 8,383,190 +18
android jsc x86 9,710,886 +26
android jsc x86_64 10,187,735 +15

Base commit: a8166bd
Branch: main

@NickGerleman NickGerleman changed the title Partial Mitigation for Samsung TextInput Hangs Mitigation for Samsung TextInput Hangs Jan 26, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42721684

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Jan 26, 2023
Summary:
Pull Request resolved: facebook#35967

In facebook#35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in the most common cases, when it is sufficient to set `android:textSize`. In simple cases, it causes typing into the TextInput to no longer hang.

This does not resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We could potentially do further work to reduce the number of spans emitted in these scenarios, but this may be fighting a losing battle against the platform.

Changelog:
[Android][Fixed] - Partial Mitigation for Samsung TextInput Hangs (Paper)

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: 1b6935014c8055c3bd547c7ba5294bb9975479c2
@NickGerleman
Copy link
Contributor Author

FYI to anyone observing, I pushed a new version of this which should be better than the first.

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Jan 26, 2023
Summary:
Pull Request resolved: facebook#35967

In facebook#35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: ec15bf996a6918cadbb53f9d371281e83bb6208e
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42721684

Summary:
Pull Request resolved: facebook#35967

In facebook#35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: 000a8ceb0200abb19a2aa77cdb78276d289c3115
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D42721684

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in be69c8b.

@rikur
Copy link

rikur commented Jan 29, 2023

Thank you for trying to fix the multiline ANRs on Samsung devices with Android 13. I built with this patch, however, and our app still hangs after a while of typing.

cipolleschi pushed a commit that referenced this pull request Jan 30, 2023
Summary:
Pull Request resolved: #35967

In #35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
kelset pushed a commit that referenced this pull request Jan 30, 2023
Summary:
Pull Request resolved: #35967

In #35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
dmytrorykun pushed a commit that referenced this pull request Jan 30, 2023
Summary:
Pull Request resolved: #35967

In #35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
dmytrorykun pushed a commit that referenced this pull request Jan 30, 2023
Summary:
Pull Request resolved: #35967

In #35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
diegolmello pushed a commit to RocketChat/react-native that referenced this pull request Feb 2, 2023
Summary:
Pull Request resolved: facebook#35967

In facebook#35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Pull Request resolved: facebook#35967

In facebook#35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.

This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.

This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.

Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.

Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs

Reviewed By: cortinico

Differential Revision: D42721684

fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
@cipolleschi cipolleschi mentioned this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants