fix emoji in iOS crash #30851

dansunhappy · 2019-04-10T12:11:42Z

Description

*fix in iOS paste emoji crash

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See Test Coverage.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
I The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

googlebot · 2019-04-10T12:11:45Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

dansunhappy · 2019-04-10T15:31:50Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).

The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.

The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

I signed it!

dnfield · 2019-04-10T16:50:34Z

Is there an issue open to discuss this?

This will need tests.

dnfield · 2019-04-10T16:51:16Z

Also, the CLA bot won't recognize your signed CLA because the commits were authored by a different email address than you're signed in with right now.

dnfield · 2019-04-10T16:51:56Z

packages/flutter/lib/src/services/text_editing.dart

  /// The text before this range.
  String textBefore(String text) {
    assert(isNormalized);
+    if(text.isNotEmpty && start > 0 && start < text.length ){


nit: spacing should be if (text.isNotEmpty && start > 0 && start < text.length) {

dnfield · 2019-04-10T16:53:05Z

packages/flutter/lib/src/services/text_editing.dart

  String textBefore(String text) {
    assert(isNormalized);
+    if(text.isNotEmpty && start > 0 && start < text.length ){
+      final int startCodeUnit = text.codeUnitAt(start);


This seems dangerous. What if the character is more than a single code unit?

https://en.wikipedia.org/wiki/UTF-16 @dnfield one character may be two code unit.

dnfield · 2019-04-10T16:54:13Z

packages/flutter/lib/src/services/text_editing.dart

    assert(isNormalized);
+    if(text.isNotEmpty && start > 0 && start < text.length ){
+      final int startCodeUnit = text.codeUnitAt(start);
+      if( startCodeUnit & 0x8C00 == 0x8C00 ){


Same spacing nit as above. This also seems dangerous - Why are we checking for these specific bits to be set on the first code unit? Are we certain that this impacts all unicode characters with those bits set? Could we somehow document what this magic number means?

this magic number is utf-16 encoding

googlebot · 2019-04-11T03:53:26Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

dansunhappy · 2019-04-11T04:39:05Z

Is there an issue open to discuss this?

This will need tests.
crash：

In TextField Repeatedly paste emoji
crash stack

2019-04-10 16:23:20.697906+0800 Runner[5581:340646] [Bugly]  Trapped uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid JSON message, decoding failed' 
(
	0   CoreFoundation                      0x0000000186210530 <redacted> + 252
	1   libobjc.A.dylib                     0x00000001853eb9f8 objc_exception_throw + 56
	2   CoreFoundation                      0x000000018612a148 <redacted> + 0
	3   Foundation                          0x0000000186bed1c8 <redacted> + 112
	4   Flutter                             0x0000000102f9cb38 -[FlutterJSONMessageCodec decode:] + 472
	5   Flutter                             0x0000000102f9ceb8 -[FlutterJSONMethodCodec decodeMethodCall:] + 68
	6   Flutter                             0x0000000102f9bc90 __45-[FlutterMethodChannel setMethodCallHandler:]_block_invoke + 56
	7   Flutter                             0x0000000102fbae9c _ZNK5shell21PlatformMessageRouter21HandlePlatformMessageEN3fml6RefPtrIN5blink15PlatformMessageEEE + 224
	8   Flutter                             0x0000000102fbefc0 _ZN5shell15PlatformViewIOS21HandlePlatformMessageEN3fml6RefPtrIN5blink15PlatformMessageEEE + 40
	9   Flutter                             0x000000010301783c _ZNSt3__110__function6__funcIZN5shell5Shell29OnEngineHandlePlatformMessageEN3fml6RefPtrIN5blink15PlatformMessageEEEE4$_27NS_9allocatorIS9_EEFvvEEclEv + 80
	10  Flutter                             0x0000000102fcbf24 _ZN3fml15MessageLoopImpl15RunExpiredTasksEv + 756
	11  Flutter                             0x0000000102fcf6c4 _ZN3fml17MessageLoopDarwin11OnTimerFireEP16__CFRunLoopTimerPS0_ + 32
	12  CoreFoundation                      0x00000001861a28f4 <redacted> + 28
	13  CoreFoundation                      0x00000001861a2624 <redacted> + 864
	14  CoreFoundation                      0x00000001861a1e58 <redacted> + 248
	15  CoreFoundation                      0x000000018619cda8 <redacted> + 1844
	16  CoreFoundation                      0x000000018619c354 CFRunLoopRunSpecific + 436
	17  GraphicsServices                    0x000000018839c79c GSEventRunModal + 104
	18  UIKitCore                           0x00000001b260fb68 UIApplicationMain + 212
	19  Runner                              0x000000010088f904 main + 124
	20  libdyld.dylib                       0x0000000185c628e0 <redacted> + 4
)

problem analysis ：
1.Cursor position calculation error while pasting，Insert emoji bytes

@dnfield

dnfield · 2019-04-11T06:21:44Z

I'm not able to reproduce this, and the crash you're seeing seems like it should probably have a fix in the iOS embedding rather than the framework.

Can you provide a sample test (maybe a driver test?) that reproduces this issue?

dnfield · 2019-04-11T06:22:08Z

(I ran a simple app on the iOS simulator with a TextField widget, and repeatedly pasted 𐐷)

dansunhappy · 2019-04-12T05:33:47Z

(I ran a simple app on the iOS simulator with a TextField widget, and repeatedly pasted 𐐷)

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel dev, v1.4.18, on Mac OS X 10.14.4 18E226, locale zh-Hans-CN)

[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[✓] iOS toolchain - develop for iOS devices (Xcode 10.2)
[✓] Android Studio (version 3.3)
[!] IntelliJ IDEA Community Edition (version 2018.3.3)
✗ Flutter plugin not installed; this adds Flutter specific functionality.
✗ Dart plugin not installed; this adds Dart specific functionality.
[✓] Connected device (2 available)
iOS 12 device，It's not an simulator
test code

in this case will crash

dnfield · 2019-04-12T23:16:08Z

What are you doing to get the cursor in the middle of the emoji?

dansunhappy · 2019-04-13T05:14:29Z

What are you doing to get the cursor in the middle of the emoji?

Random click @dnfield

songfei · 2019-04-18T01:47:03Z

@dnfield may be change iphone language to Chinese Simple ， try again.

songfei · 2019-04-18T01:58:10Z

main() {
  String a='𐐷';
  print(a.codeUnits.map((v)=> v.toRadixString(16)));
  String b='宋';
  print(b.codeUnits.map((v)=> v.toRadixString(16)));
}

https://en.wikipedia.org/wiki/UTF-16 @dnfield one character may be two code units.
try this code,

dnfield · 2019-04-18T02:07:38Z

Some are 4 code points. I'm trying to find out how to reproduce the crash though.

/Cc @GaryQian @goderbauer

flutter/engine@9e315e6...2e8e96f git log 9e315e6..2e8e96f --no-merges --oneline 2e8e96f Roll Dart to 4eb879133a06c86869dc54cecf904f4b1d46c47b (flutter/engine#6276) 765b0cc Roll src/third_party/skia 060e992ef5b8..1d6281d4bb47 (34 commits) (flutter/engine#6274) 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, who should be CC'd on the roll, and stop the roller if necessary.

Closes #18679

…e iOS version is still incomplete and will therefore require additional renaming. (#21771) (#22006)

googlebot · 2019-04-18T10:17:38Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

dansunhappy · 2019-04-18T10:29:16Z

Some are 4 code points. I'm trying to find out how to reproduce the crash though.

/Cc @GaryQian @goderbauer

1.Test code

import 'package:flutter/material.dart';

class MyTextField extends StatelessWidget {
//  MediaQuery
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: TextField(
          maxLines: null,
        ),
      ),
    );
  }
}

void main() async {
  runApp(MaterialApp(
    home: MyTextField(),
  ));
}

2. environment

device：iOS 12 ，iPhone
keyboard setting ： english & emoji

googlebot added the cla: no label Apr 10, 2019

dansunhappy closed this Apr 10, 2019

dansunhappy reopened this Apr 10, 2019

dnfield added a: typography Text rendering, possibly libtxt platform-ios iOS applications specifically labels Apr 10, 2019

dnfield reviewed Apr 10, 2019

View reviewed changes

goderbauer added the framework flutter/packages/flutter repository. See also f: labels. label Apr 10, 2019

engine-flutter-autoroll and others added 3 commits April 18, 2019 18:15

clarify how to display an image with a specific density (#21152)

81c1b25

Closes #18679

Renamed 'flutter materialize' to 'flutter make-host-app-editable'. Th…

a733b60

…e iOS version is still incomplete and will therefore require additional renaming. (#21771) (#22006)

fix emoji crash in ios

39f9cef

dansunhappy closed this Apr 23, 2019

This was referenced Apr 23, 2019

fix_emoji_crash #31468

Closed

iOS crash if platform message has invalid UTF8 #31525

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021

fix emoji in iOS crash #30851

fix emoji in iOS crash #30851

Uh oh!

Conversation

dansunhappy commented Apr 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Checklist

Breaking Change

Uh oh!

googlebot commented Apr 10, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

dansunhappy commented Apr 10, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

dnfield commented Apr 10, 2019

Uh oh!

dnfield commented Apr 10, 2019

Uh oh!

dnfield Apr 10, 2019

Choose a reason for hiding this comment

Uh oh!

dnfield Apr 10, 2019

Choose a reason for hiding this comment

Uh oh!

songfei Apr 18, 2019

Choose a reason for hiding this comment

Uh oh!

dnfield Apr 10, 2019

Choose a reason for hiding this comment

Uh oh!

songfei Apr 18, 2019

Choose a reason for hiding this comment

Uh oh!

googlebot commented Apr 11, 2019

Uh oh!

dansunhappy commented Apr 11, 2019 • edited by dnfield Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Apr 11, 2019

Uh oh!

dnfield commented Apr 11, 2019

Uh oh!

dansunhappy commented Apr 12, 2019

Uh oh!

dnfield commented Apr 12, 2019

Uh oh!

dansunhappy commented Apr 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

songfei commented Apr 18, 2019

Uh oh!

songfei commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Apr 18, 2019

Uh oh!

googlebot commented Apr 18, 2019

Uh oh!

dansunhappy commented Apr 18, 2019

1.Test code

2. environment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

dansunhappy commented Apr 10, 2019 •

edited

Loading

dansunhappy commented Apr 11, 2019 •

edited by dnfield

Loading

dansunhappy commented Apr 13, 2019 •

edited

Loading

songfei commented Apr 18, 2019 •

edited

Loading