-
Notifications
You must be signed in to change notification settings - Fork 6k
[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.