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

Fix backspace crash on Chinese devices #9734

Merged
merged 4 commits into from
Jul 10, 2019
Merged

Conversation

GaryQian
Copy link
Contributor

The Selection.extendLeft and extendRight calls on Huawei devices has a bug where it will ignore the selection indexes and always use the initial length of the editable. This works around it by falling back to a simpler algorithm.

@GaryQian
Copy link
Contributor Author

Partially fixes flutter/flutter#33642

Copy link
Member

@jason-simmons jason-simmons left a comment

Choose a reason for hiding this comment

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

LGTM

Selection.extendLeft(mEditable, mLayout);
}
} catch (IndexOutOfBoundsException e) {
// On Huaewi devices on initial app startup before focus is lost,
Copy link
Member

Choose a reason for hiding this comment

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

typo: "Huawei"

@CaiJingLong
Copy link

CaiJingLong commented Jul 10, 2019

It also happens on the xiaomi device, probably due to the security keyboard.

  1. Enter information in the username input box.
  2. Switch focus to the password input box, the system's security keyboard pops up. Enter the password.
  3. Switch focus to the username input box, delete 1~2 characters, crash.

My record screen video : https://drive.google.com/file/d/1jJRTjHvXu1bmSaannWfEtlGD8Q-VTMoJ/view?usp=sharing

And I close the security keyboard, no crash.

No crash video: https://drive.google.com/file/d/1tUrJoEptlSCjAUThqUvJ3PHtePczpFLD/view?usp=sharing

@GaryQian
Copy link
Contributor Author

Either way, this should fix the crashing. It turns out it may not be unique to only Huawei, as there were a few reports on xiaomi devices.

@GaryQian GaryQian changed the title Fix backspace crash on Huawei devices Fix backspace crash on Chinese devices Jul 10, 2019
@GaryQian
Copy link
Contributor Author

The root bug is actually in the affected devices. This is a workaround. Until the root bug is fixed, I'm not sure if we can easily reach 100% correct backspace behavior here. Once the app loses focus for the first time, the problem is fixed.

When calling extendLeft and extendRight, it seems that the implementation under some conditions fails to update some internal index or buffer, causing it to perform all extend (and move) operations based off of the initial condition of the text, completely ignoring any selection changes. When the text becomes shorter than the initial contents, the crash comes from trying to access text that no longer exists.

Since the Android API is not doing what it promises, and the bug prevents working with selections to fix it, I dont think a very straightforwards full solution exists. Attempts at handling this explicitly result in also breaking emoji backspace support.

@GaryQian GaryQian merged commit fa7627d into flutter:master Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 10, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 11, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Jul 11, 2019
flutter/engine@75387db...49445ce

git log 75387db..49445ce --no-merges --oneline
49445ce FLEViewController/Engine API changes (flutter/engine#9750)
2a79462 Add Fuchsia build CI presubmit steps (flutter/engine#9736)
67cebdb Roll fuchsia/sdk/core/linux-amd64 from KGmm_RIJoXS19zTm2crjM3RYpmp5Y03-fLUeVdylbTYC to ehWVT9QJbC-vFMM6SkkQM9HJ9oITFCws7FC9JnrFq2gC (flutter/engine#9765)
089c740 Roll fuchsia/sdk/core/mac-amd64 from lCQWEeR_Ert7t_qAbMRycwrRyZC-dIprYPyPJzwPmg4C to EYnRdXFT9l-d8Qkz4zeTRXnqfV3KQzpQhoPs1r0-740C (flutter/engine#9759)
b22410e Include SkParagraph headers only when the enable-skshaper flag is on (flutter/engine#9758)
2cd650d Minimal integration with the Skia text shaper module (flutter/engine#9556)
f775f5e Re-enable the Wuffs GIF decoder (flutter/engine#9466)
aca0482 Make all shell unit tests use the OpenGL rasterizer. (flutter/engine#9746)
bc57291 Make FLEViewController's view an internal detail (flutter/engine#9741)
9776043 Synchronize main thread and gpu thread for first render frame (flutter/engine#9506)
f600ae8 Use libc++ variant of string view and remove the FML variant. (flutter/engine#9737)
564f53f Revert "Improve caching limits for Skia (#9503)" (flutter/engine#9740)
b453d3c libtxt: fix reference counting of SkFontStyleSets held by font asset providers (flutter/engine#9561)
fa7627d Fix backspace crash on Chinese devices (flutter/engine#9734)
56885f7 Let pushColorFilter accept all types of ColorFilters (flutter/engine#9641)
6dccb21 Roll src/third_party/skia 96fdfe0fe88e..af4e7b6cf616 (1 commits) (flutter/engine#9735)
8511d9b Roll fuchsia/sdk/core/mac-amd64 from byM-kyxL4bemlTYNqhKUfJfZoIUrCSzS6XzsFr4n9-MC to lCQWEeR_Ert7t_qAbMRycwrRyZC-dIprYPyPJzwPmg4C (flutter/engine#9742)
b3bf0a1 Roll fuchsia/sdk/core/linux-amd64 from I2Qe1zxgckzIzMBTztvzeWYsDgcb9Fw-idSI16oIlx8C to KGmm_RIJoXS19zTm2crjM3RYpmp5Y03-fLUeVdylbTYC (flutter/engine#9743)
7e56823 Fix windows test by not attempting to open a directory as a file. (flutter/engine#9745)
6cf0d13 Roll src/third_party/skia a3ffaabcc4f2..96fdfe0fe88e (5 commits) (flutter/engine#9731)
49a00ae Fix Fuchsia build. (flutter/engine#9730)
b3bb39b Roll src/third_party/dart 06c3d7ad3a...09fc76bc51 (flutter/engine#9728)
2284210 Make the license script compatible with recently changed Dart I/O stream APIs (flutter/engine#9725)
ad582b5 Rework image & texture management to use concurrent message queues. (flutter/engine#9486)
1dcd5f5 Roll src/third_party/skia 6b82cf638682..a3ffaabcc4f2 (24 commits) (flutter/engine#9726)
129979c Revert "Roll src/third_party/dart 06c3d7ad3a..7acecda2cc (12 commits)" (flutter/engine#9724)
8020d7e Roll src/third_party/skia 56065d9b875f..6b82cf638682 (3 commits) (flutter/engine#9718)
e24bd78 Roll src/third_party/dart 06c3d7ad3a..7acecda2cc (12 commits)
3d2668c Reland isolate group changes
802bd15 iOS platform view opacity (flutter/engine#9667)
3b6265b Roll src/third_party/dart b5aeaa6796..06c3d7ad3a (44 commits)
887e052 Refactor ColorFilter to have a native wrapper (flutter/engine#9668)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
tvolkert pushed a commit to tvolkert/engine that referenced this pull request Jul 18, 2019
johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
flutter/engine@75387db...49445ce

git log 75387db..49445ce --no-merges --oneline
49445ce FLEViewController/Engine API changes (flutter/engine#9750)
2a79462 Add Fuchsia build CI presubmit steps (flutter/engine#9736)
67cebdb Roll fuchsia/sdk/core/linux-amd64 from KGmm_RIJoXS19zTm2crjM3RYpmp5Y03-fLUeVdylbTYC to ehWVT9QJbC-vFMM6SkkQM9HJ9oITFCws7FC9JnrFq2gC (flutter/engine#9765)
089c740 Roll fuchsia/sdk/core/mac-amd64 from lCQWEeR_Ert7t_qAbMRycwrRyZC-dIprYPyPJzwPmg4C to EYnRdXFT9l-d8Qkz4zeTRXnqfV3KQzpQhoPs1r0-740C (flutter/engine#9759)
b22410e Include SkParagraph headers only when the enable-skshaper flag is on (flutter/engine#9758)
2cd650d Minimal integration with the Skia text shaper module (flutter/engine#9556)
f775f5e Re-enable the Wuffs GIF decoder (flutter/engine#9466)
aca0482 Make all shell unit tests use the OpenGL rasterizer. (flutter/engine#9746)
bc57291 Make FLEViewController&flutter#39;s view an internal detail (flutter/engine#9741)
9776043 Synchronize main thread and gpu thread for first render frame (flutter/engine#9506)
f600ae8 Use libc&flutter#43;&flutter#43; variant of string view and remove the FML variant. (flutter/engine#9737)
564f53f Revert &flutter#34;Improve caching limits for Skia (flutter#9503)&flutter#34; (flutter/engine#9740)
b453d3c libtxt: fix reference counting of SkFontStyleSets held by font asset providers (flutter/engine#9561)
fa7627d Fix backspace crash on Chinese devices (flutter/engine#9734)
56885f7 Let pushColorFilter accept all types of ColorFilters (flutter/engine#9641)
6dccb21 Roll src/third_party/skia 96fdfe0fe88e..af4e7b6cf616 (1 commits) (flutter/engine#9735)
8511d9b Roll fuchsia/sdk/core/mac-amd64 from byM-kyxL4bemlTYNqhKUfJfZoIUrCSzS6XzsFr4n9-MC to lCQWEeR_Ert7t_qAbMRycwrRyZC-dIprYPyPJzwPmg4C (flutter/engine#9742)
b3bf0a1 Roll fuchsia/sdk/core/linux-amd64 from I2Qe1zxgckzIzMBTztvzeWYsDgcb9Fw-idSI16oIlx8C to KGmm_RIJoXS19zTm2crjM3RYpmp5Y03-fLUeVdylbTYC (flutter/engine#9743)
7e56823 Fix windows test by not attempting to open a directory as a file. (flutter/engine#9745)
6cf0d13 Roll src/third_party/skia a3ffaabcc4f2..96fdfe0fe88e (5 commits) (flutter/engine#9731)
49a00ae Fix Fuchsia build. (flutter/engine#9730)
b3bb39b Roll src/third_party/dart 06c3d7ad3a...09fc76bc51 (flutter/engine#9728)
2284210 Make the license script compatible with recently changed Dart I/O stream APIs (flutter/engine#9725)
ad582b5 Rework image & texture management to use concurrent message queues. (flutter/engine#9486)
1dcd5f5 Roll src/third_party/skia 6b82cf638682..a3ffaabcc4f2 (24 commits) (flutter/engine#9726)
129979c Revert &flutter#34;Roll src/third_party/dart 06c3d7ad3a..7acecda2cc (12 commits)&flutter#34; (flutter/engine#9724)
8020d7e Roll src/third_party/skia 56065d9b875f..6b82cf638682 (3 commits) (flutter/engine#9718)
e24bd78 Roll src/third_party/dart 06c3d7ad3a..7acecda2cc (12 commits)
3d2668c Reland isolate group changes
802bd15 iOS platform view opacity (flutter/engine#9667)
3b6265b Roll src/third_party/dart b5aeaa6796..06c3d7ad3a (44 commits)
887e052 Refactor ColorFilter to have a native wrapper (flutter/engine#9668)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants