Track and use fallback TextAffinity for null affinity platform TextSelections. #44622
Conversation
fluttergithubbot
added
the
framework
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Will attempt to do this without the addition of |
GaryQian
changed the title
|
This version makes use of null instead of the explicit "ambiguous" affinity. This adds no additional API, and is no longer dependent on flutter/engine#13787 |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
The code looks good and makes sense to me, but did you verify that it fixes the problem in the issue? I still get the problem when I try it. Maybe I'm doing something else though.
There was a problem hiding this comment.
Would it be more robust to use value.copyWith here, or did you purposely avoid it?
There was a problem hiding this comment.
Ahh you are right, should just use this.
|
Wait it was working in my repro, let me try some more cases. |
|
@justinmc are you sure you are running the parched version? Mine seems to work with a variety of TextFields and settings on multiple devices. Do you mind posting the code you are using? |
|
There is no engine piece needed now, right? I just checked out your branch and ran a plain app with a multiline TextField. |
|
I'm trying it again and will post my code in a second. |
|
Oh wait this may not work for iOS that's what it probably is |
|
This specifically addresses the fact that android does not pass an affinity. Have not seen what iOS does. |
|
Give me a second and I'll look at the iOS side of it |
|
Ohh that makes sense. Sounds good. |
GaryQian
force-pushed
the
affinity
branch
2 times, most recently
from
7846c21 to
1fbf174
Compare
|
@justinmc Should be ready for another round. This version is a bit cleaner and fixes the bug on iOS in conjunction with flutter/engine#13852 |
There was a problem hiding this comment.
Avoid "we" statements in comments. They're confusing to non-native speakers and require extra thought to understand who "we" is referring to. (And they don't really add any information anyhow)
* flutter/engine@a677925 Cherry pick infra testing updates into hotfixes * flutter/engine@b1bb773 Raise API level for reportFullyDrawn * flutter/engine@a433ed9 v1.12.13+hotfix.7 cherry-picks * google/skia@a640745 Disable QCOM_tiled_rendering while we wait for test devices * flutter/engine@bcb8267 Revert "Do not default to downstream affinity on iOS insertText" * d345a3b Revert "Track and use fallback TextAffinity for null affinity platform TextSelections. (#44622)" * bd25f70 Upgrade dartdoc to 0.29.3 * 7915e58 Change firebase Pixel3 version from Q-beta3 to 29
* flutter/engine@a677925 Cherry pick infra testing updates into hotfixes * flutter/engine@b1bb773 Raise API level for reportFullyDrawn * flutter/engine@a433ed9 v1.12.13+hotfix.7 cherry-picks * google/skia@a640745 Disable QCOM_tiled_rendering while we wait for test devices * flutter/engine@bcb8267 Revert "Do not default to downstream affinity on iOS insertText" * d345a3b Revert "Track and use fallback TextAffinity for null affinity platform TextSelections. (flutter#44622)" * bd25f70 Upgrade dartdoc to 0.29.3 * 7915e58 Change firebase Pixel3 version from Q-beta3 to 29
5 participants
Fixes #40095
When affinity is unknown, we pass null instead of defaulting to
TextAffnity.downstream. This allows us to supply fallback affinities computed from libtxt.Since LibTxt tracks affinities in the
getPositionForOffsetcall (used in renderEditable inselectPositionAtandselectWordEdge), we can use the affinity information by falling back to libtxt affinity when the platform provided affinity is null.This also exposes
getLineBoundaryinTextPainter, which is then used to choose the LibTxt affinity when at the end of the line since selections at the end of lines cannot be downstream affinity without jumping to the next line.