-
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 text range logic for a11y #16496
Conversation
@@ -28,3 +28,7 @@ | |||
EXPECT_EQ(result.location, 0UL); | |||
EXPECT_EQ(result.length, 0UL); | |||
} | |||
|
|||
TEST(StringRangeSanitizationTest, HandlesEndOfRange) { | |||
EXPECT_EQ(fml::RangeForCharacterAtIndex(@"1234", 4).location, 4UL); |
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.
please add assert for the NSRange.length too
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.
Done
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!
* c932214 Ensure fields of Rect and OffsetBase classes are optimized as non-null. (flutter/engine#16465) * 5c70356 Simplify task queues locking mechanism (flutter/engine#16477) * d589dde Fix text range logic for a11y (flutter/engine#16496) * 1a4f4e3 Fix unused import in Android embedder (flutter/engine#16494) * 964ae10 Disable ShellTest.WaitForFirstFrameTimeout on Fuchsia (flutter/engine#16495) * 6158f03 Roll src/third_party/skia 87e3bef6f82f..9f3eef796f63 (7 commits) (flutter/engine#16493)
Make sure that a text range at the end of the string is still valid.
This reverts commit 7866595.
see also: #8747
The linked PR unintentionally introduced a bug in text range handling where we treated a text range with a location at the end of the string as invalid. This should not be the case.
This resulted in a crash in a11y on iOS when you entered text, the cursor is at the end of the field, and you tap.
This app can reproduce the issue with VoiceOver on:
Fortunately, the FML change doesn't break any of the original tests - I think it was just an oversight at the time, the other FML method handles cases like this correctly.
/cc @jkurtw @mehmetf @chingjun - this fixes b/149077991