-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix getLineBoundary for TextAffinity #28008
Conversation
cfea916 to
fdf33e6
Compare
fdf33e6 to
0c92a2b
Compare
lib/ui/text.dart
Outdated
|
|
||
| // _getLineBoundary only considers the offset and assumes that the | ||
| // TextAffinity is upstream. In the case that TextPosition is just after a | ||
| // wordwrap (downstream), we need to get the line for the next offset. |
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.
If the line ends with a RTL run then the _getLineBoundary function will still report the same line for position.offset + 1 I think?
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.
Oh nvm _getLineBoundary reports the logical start and end of a line so +1 can't be on the same line, from the tests it looks like position.offset + 1 works even if it exceeds the text length.
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.
Yeah that's what I'm seeing, but I'm glad you brought this up. I just took a dive into Flutter RTL keyboard support while investigating this and it seems very broken. I don't know anything about any RTL languages unfortunately, but the behavior is very different from the browser. I will probably create a new issue for that.
c0135ba to
ea255c7
Compare
|
Nevermind, that was red due to a real edge case that I fixed. Merging. |
019fbbf to
794ca14
Compare
Fix a bug in a calculation when selection is at a wordwrap
Paragraph.getLineBoundary accepts a TextPosition, but it disregards the included TextAffinity. This causes it to give the wrong answer at downstream wordwrap boundaries (see the linked issue for an example).
This PR makes it give the right response for all TextAffinities.
Fixes flutter/flutter#87970