-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[CP] Workaround iOS text input crash for emoji+Korean text #36621
[CP] Workaround iOS text input crash for emoji+Korean text #36621
Conversation
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
Changes LGTM. Are the tests supposed to pass without infra failures? |
https://fuchsia.googlesource.com/third_party clones failed? @CaseyHillers what's up here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I don't understand either. It looks related to #34720, possibly a corrupted bot cache. I'll retry the linux builds. |
nit: Can we update the PR title to be something more desciptive? A common format is |
@cyanglaz Hello. Thanks a lot for the fix and sorry to bother again 🙏 This issue is getting serious in my country, South Korea. Would there be timeline to merge this and release a hot-fix? |
|
@cyanglaz could you push an empty commit to rerun tests. |
I re-ran manually 🤞 |
Gold has detected about 1 new digest(s) on patchset 2. |
@cyanglaz this doesn't build, does this also need to cherry-pick #34508? Can you validate the fix on the candidate branch? |
Or at least the |
Having some issue with local build and im trying to fix it. Meanwhile, im pushing the |
Gold has detected about 4 new digest(s) on patchset 3. |
I'm not sure what this failure is. Still trying to build local engine from the release branch, got:
Not sure if i need to do something like flutter/flutter#96745 |
Thanks for your hard work 👍 Just FYI: This issue still happens in 16.1 beta 5. |
I didn't see
Not sure why it's missing from the logs, but I reproduced the test failure locally:
When I cherry-picked in #34508 to see if
@cyanglaz can you try to step through these and see why these tests are failing? |
@cyanglaz and I are actively debugging this right now, can we hold on the release hotfix until this is complete? |
Sounds good, please let me know when resolved to start the release. |
I think I hadn't built the engine properly on both changes; it does seem to pass if #34508 is cherry-picked before #36295. However I don't have permission to push to your fork, @cyanglaz, can you try that? |
9a5983e
to
c92f1ca
Compare
The tests pass on #36807 which means it should also pass on this one https://ci.chromium.org/p/flutter/builders/try/Mac%20Unopt/19879 since they are now the same. |
Closing in favor of #36807 |
Hot fixing: flutter/flutter#111494
PR: #34508, #36295
CP issue: flutter/flutter#112963
Pre-launch Checklist
writing and running engine tests.
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.