-
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
Fix macOS text composing #49314
Fix macOS text composing #49314
Conversation
6e8277f
to
26afd05
Compare
26afd05
to
51b260f
Compare
cursor_pos += active_model_->composing_range().start(); | ||
active_model_->UpdateComposingText(text); | ||
active_model_->SetSelection(TextRange(cursor_pos, cursor_pos)); | ||
active_model_->UpdateComposingText(text, TextRange(cursor_pos, cursor_pos)); |
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.
Not really related to this change, but isn't the composing text being added twice 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.
You mean between AddText
as well as UpdateComposingText
? It sort of looks like it, though perhaps we're relying on a side effect of one or the other?
a1b4044
to
ecf8375
Compare
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.
cursor_pos += active_model_->composing_range().start(); | ||
active_model_->UpdateComposingText(text); | ||
active_model_->SetSelection(TextRange(cursor_pos, cursor_pos)); | ||
active_model_->UpdateComposingText(text, TextRange(cursor_pos, cursor_pos)); |
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.
You mean between AddText
as well as UpdateComposingText
? It sort of looks like it, though perhaps we're relying on a side effect of one or the other?
Fixes flutter/flutter#68547 (almost, this requires an oneliner framework patch to work). The selection wasn't being updated correctly before this so there were no highlights. Now the selection is properly highlighted (although it seems the styling has been updated in sonoma but it's better than offering no visual feedback): https://github.com/flutter/engine/assets/31859944/ab8557f4-7215-46de-8a9d-c2e3982695eb [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
…141217) flutter/engine@8c1501f...542fea9 2024-01-09 31859944+LongCatIsLooong@users.noreply.github.com Fix macOS text composing (flutter/engine#49314) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC bdero@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes #68547 for macOS. Needs flutter/engine#49314
Fixes flutter/flutter#68547 (almost, this requires an oneliner framework patch to work).
The selection wasn't being updated correctly before this so there were no highlights. Now the selection is properly highlighted (although it seems the styling has been updated in sonoma but it's better than offering no visual feedback):
Screen.Recording.2023-12-21.at.12.00.38.AM.mov
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.