8342233: Regression: TextInputControl selection is backwards in RTL mode #1609
Conversation
into ag.rtl.regression.2
…into ag.rtl.regression.2
|
👋 Welcome back angorya! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
|
| return getInsertionPoint(paragraphNode, | ||
| x - paragraphNode.getLayoutX(), | ||
| y - paragraphNode.getLayoutY()); | ||
| if ( |
There was a problem hiding this comment.
I really don't like empty opening brackets in conditions but i guess it is not explicitly prohibited. Aside of that fix looks reasonable.
|
@lukostyra Can you be the "R"eviewer on this? |
| @@ -814,23 +828,20 @@ private char getCharacter(int index) { | |||
| @Override | |||
| protected int getInsertionPoint(double x, double y) { | |||
| TextArea textArea = getSkinnable(); | |||
| Text paragraphNode = getTextNode(); | |||
| Text n = getTextNode(); | |||
There was a problem hiding this comment.
What's the reason for the name change paragraphNode -> n?
There was a problem hiding this comment.
The change made the following if statement easier to digest fit on a single line.
It's a local variable within a short function, I think it should be ok.
A fix for JDK-8319844 Text/TextFlow.hitTest() introduced a regression in the
TextArea/TextField/PasswordFieldin the RTL mode.The fix is to flip the x coordinates when needed in the
TextAreaSkin/TextFieldSkin.The RTL node orientation also breaks navigation using keyboard arrow keys, but that's a different issue: JDK-8296266.
I tried to devise a headful test, but it is currently blocked by JDK-8189167
The fix can be tested manually using the Monkey Tester, with the headful test to be added probably as a part of JDK-8326869 .
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1609/head:pull/1609$ git checkout pull/1609Update a local copy of the PR:
$ git checkout pull/1609$ git pull https://git.openjdk.org/jfx.git pull/1609/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1609View PR using the GUI difftool:
$ git pr show -t 1609Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1609.diff
Using Webrev
Link to Webrev Comment