openjdk
8330559: Trailing space not rendering correctly in TextFlow in RTL mode
Fix trailing space present for complex text ( LTR text with RTL text ) Example: "Arabic: العربية"
Added case to handle complex text in getPosX of TextRun
Tested the changes with the code present in the bug.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8330559: Trailing space not rendering correctly in TextFlow in RTL mode (Bug - P3)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1988/head:pull/1988
$ git checkout pull/1988
Update a local copy of the PR:
$ git checkout pull/1988
$ git pull https://git.openjdk.org/jfx.git pull/1988/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1988
View PR using the GUI difftool:
$ git pr show -t 1988
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1988.diff
Using Webrev
:wave: Welcome back zelmidaoui! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@Ziad-Mid This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8330559: Trailing space not rendering correctly in TextFlow in RTL mode
Reviewed-by: angorya
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
The fix looks good in the reproducer, good job!
I have less luck in the monkey tester where we hit other issues (JDK-8318095 comes to mind). I want to test this change more, and perhaps ask @nlisker to take a look, since I am less exposed to RTL in my environment.
@Ziad-Mid do you think this fix is independent of (JDK-8318095 , in other words, would fixing (JDK-8318095 will require undoing changes in this PR?
Updated reproducer: https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/bugs/TextFlow_ExtraSpace_8330559.java
Can this be tested with a JUnit Test? I'm asking as there are quite some problems right now with complex text, fonts and RTL. So every regression test will help in the future
@Ziad-Mid do you think this fix is independent of (JDK-8318095
I am checking the JDK-8318095 with more tests, but a first thought they look independant to me. I will check both and let you know.
Can this be tested with a JUnit Test?
I am working on a JUnit Test for this bug as suggested .
@andy-goryachev-oracle I have provided a fix for JDK-8318095 in PR 1995. It doesn't impact the changes in this PR as mentioned, you can review it and let me know your suggestions for both PRs
@Ziad-Mid Pushed as commit 14305d623144934f8b9c88e1ddf434ab51902857.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.