-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
[ios][ios17][text_input]fix text input system highlight in iOS 17 Beta 7 with firstRectForRange #45303
Conversation
9b7de88
to
a01f9b3
Compare
} | ||
} | ||
// TODO(hellohaunlin): Remove iOS 17 check. The logic should also work for older versions. | ||
if (@available(iOS 17, *)) { |
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 can remove this one now; CGRectIsNull will always be true for <17 because of the check you already have above.
BOOL endsOnOrAfterEndOfRange = _selectionRects[i].position >= end - 1; // end is exclusive | ||
BOOL nextSelectRectIsOnNextLine = | ||
!isLastSelectionRect && | ||
_selectionRects[i + 1].rect.origin.y != _selectionRects[i].rect.origin.y; |
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.
Is every character on the same line guaranteed to have the same y
origin, or could this have false positives for unusual characters?
490df64
to
d532b47
Compare
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
d532b47
to
8a4ce0f
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.
LGTM with typo fix. Thanks for adding all the additional test coverage for the differing height and overlapping line cases!
BOOL endsOnOrAfterEndOfRange = _selectionRects[i].position >= end - 1; // end is exclusive | ||
BOOL nextSelectionRectIsOnNextLine = | ||
!isLastSelectionRect && | ||
// Seleciton rects from different langauges in 2 lines may overlap with each other. |
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.
Typo: Selection
update for varying selection rect size
8a4ce0f
to
d6b85f9
Compare
The golden file changes seem to be a false positive. |
…iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303)
…iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303)
…iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303)
…sions) (#133924) Manual roll requested by zra@google.com flutter/engine@489c399...e496eec 2023-09-02 skia-flutter-autoroll@skia.org Roll Skia from 2d8849f9f0cc to 15f77147a3ec (1 revision) (flutter/engine#45414) 2023-09-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from OF4TS05qlWCjukWw6... to MesZPNdj-uw8VdCyV... (flutter/engine#45413) 2023-09-02 dkwingsmt@users.noreply.github.com Remove --disable-service-auth-codes (flutter/engine#45356) 2023-09-02 bdero@google.com [Impeller] Import cstring for memcpy. (flutter/engine#45408) 2023-09-02 skia-flutter-autoroll@skia.org Roll Dart SDK from cdf1ce0c6d7e to a5c7102af509 (1 revision) (flutter/engine#45412) 2023-09-02 skia-flutter-autoroll@skia.org Roll ANGLE from 179bd7762ffa to ebf1e7163216 (1 revision) (flutter/engine#45411) 2023-09-02 dkwingsmt@users.noreply.github.com Remove deprecated MOCK_METHODx calls (flutter/engine#45307) 2023-09-02 jonahwilliams@google.com [Impeller] Better demonstrate blur and draw picture? (flutter/engine#45388) 2023-09-02 jonahwilliams@google.com [Impeller] Make paths externally immutable, update all tests to use PathBuilder to create Path. (flutter/engine#45393) 2023-09-02 skia-flutter-autoroll@skia.org Roll ANGLE from 962fdf7b7882 to 179bd7762ffa (1 revision) (flutter/engine#45409) 2023-09-02 flar@google.com Cull the RTree bounds when they are forwarded in DrawDisplayList (flutter/engine#45358) 2023-09-02 skia-flutter-autoroll@skia.org Roll Skia from fedff79a6afc to 2d8849f9f0cc (3 revisions) (flutter/engine#45407) 2023-09-02 jonahwilliams@google.com [impeller] premultiply vertices colors. (flutter/engine#45406) 2023-09-01 skia-flutter-autoroll@skia.org Roll ANGLE from 6a09e41ce6ea to 962fdf7b7882 (224 revisions) (flutter/engine#45400) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 22ae23891e8e to fedff79a6afc (1 revision) (flutter/engine#45405) 2023-09-01 30870216+gaaclarke@users.noreply.github.com [Impeller] turned on validations for all debug builds (flutter/engine#45350) 2023-09-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from sk7JBGzW1Jw10Wy-T... to OF4TS05qlWCjukWw6... (flutter/engine#45403) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 2c0405489966 to 22ae23891e8e (1 revision) (flutter/engine#45402) 2023-09-01 737941+loic-sharma@users.noreply.github.com [Windows] Update vsync on raster thread (flutter/engine#45310) 2023-09-01 skia-flutter-autoroll@skia.org Roll Dart SDK from a2ea759c16cc to cdf1ce0c6d7e (1 revision) (flutter/engine#45397) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from f3f6c733c7e6 to 2c0405489966 (1 revision) (flutter/engine#45396) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 02fa14799c6c to f3f6c733c7e6 (1 revision) (flutter/engine#45394) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from d5d3b0d4ee77 to 02fa14799c6c (2 revisions) (flutter/engine#45392) 2023-09-01 41930132+hellohuanlin@users.noreply.github.com [ios][ios17][text_input]fix text input system highlight in iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from d6266ef14a7e to d5d3b0d4ee77 (2 revisions) (flutter/engine#45389) 2023-09-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 0c121a6431cc to a2ea759c16cc (1 revision) (flutter/engine#45384) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from sk7JBGzW1Jw1 to MesZPNdj-uw8 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 aaclarke@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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
… 17 Beta 7 with firstRectForRange (#45398) original PR: #45303 *Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.* *List which issues are fixed by this PR. You must list at least one issue.* flutter/flutter#131622 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Design doc with more details: https://docs.google.com/document/d/1sM3HMv-SQin39yX1aPUU7vtGv7Hcef1Quc3QhRXBl6A/edit?resourcekey=0-SFYD8vmOIkXiXCZvB1Wlcw#heading=h.tul5o3hopauh
Just to be safe, for now we only change for iOS 17. However, the same logic should apply to older iOS as well. So will update in a separate PR.
Basically we were using
firstRectForRange
incorrectly. According to the API doc,firstRectForRange
should return the rect for the queried range, or a subrange through the end of line, if the range encompasses multiple lines.For LTR language:
Query:
[1, 2, 3, 4]
Returns the following rect denoted by
[ ]
:Similarly, for RTL language (Arabic):
Query:
[1, 2, 3, 4]
Returns the following rect denoted by
[ ]
:List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#131622
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.