Skip to content
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 test when a Skia RTL positioning bug is fixed #118902

Closed

Conversation

justinmc
Copy link
Contributor

@justinmc justinmc commented Jan 20, 2023

See this comment for where this came about.

There is an existing bug about the cursor going to the penultimate character in RTL text (#118566), which is being fixed in Skia. Once that fix is rolled into Flutter, the test that's modified in this PR will fail, and this PR will be needed to make it pass again.

This must not be merged until Skia fix https://skia-review.googlesource.com/c/skia/+/619838/1 is rolled into Flutter.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Jan 20, 2023
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once SKia change has rolled in and tests pass.

jason-simmons added a commit to jason-simmons/flutter that referenced this pull request Jan 31, 2023
…roll

https://skia-review.googlesource.com/c/skia/+/619838 fixes a bug in
SkParagraph.  One of the selectable region tests relies on the old
behavior.

After that Skia change rolls into the framework, the update to the test
in flutter#118902 should be landed.
auto-submit bot pushed a commit that referenced this pull request Jan 31, 2023
…roll (#119653)

https://skia-review.googlesource.com/c/skia/+/619838 fixes a bug in
SkParagraph.  One of the selectable region tests relies on the old
behavior.

After that Skia change rolls into the framework, the update to the test
in #118902 should be landed.
@goderbauer
Copy link
Member

This appears to be the same as #119756. This one can probably be closed if the other more uptodate one lands?

@justinmc
Copy link
Contributor Author

justinmc commented Feb 6, 2023

Sounds good, this is probably redundant now that that one was merged.

@justinmc justinmc closed this Feb 6, 2023
@justinmc justinmc deleted the rtl-penultimate-character-test-fix branch February 6, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants