RFR: 8296266: TextArea: Navigation breaks with RTL text

[Andy Goryachev]

On Mon, 9 Oct 2023 22:14:10 GMT, Phil Race <prr at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextAreaSkin.java line 572:
>> 
>>> 570:         if (isRTL()) {
>>> 571:             // Text node is mirrored.
>>> 572:             moveRight = !moveRight;
>> 
>> This method confuses me as to what it actually wants to do and why
>> As in, "move right, no, your other right" (!)
>> 
>> isRTL() checks the NodeOrientation so it seems in such a case "right" means "left".
>> Have you checked the fix with both orientations ?
>> Seems to me it reverses the meaning of the arrow keys without regard to the text.
>> 
>> And he BreakIterator is getting the next logical position, but this method claims to move to the next visual position.
>> The method is called nextCharacterVISUALLY, but with this code its actually moving LOGICALLY,
>> so I'm not seeing how this works for mixed directional text for either setting of "isRTL()" which seems to me to be very questionable to even check here. IMO the arrow key should work the same way no matter whether the UI is laid out with nodes from RTL or LTR.
>> 
>> Perhaps the word "Visually" was not the best choice of name here ? And "right" is questionable too. But I'm not 100% sure.
>> if you really do want LOGICALLY, then this code will probably work, except I find the isRTL() questionable even then.
>> 
>> What exactly was wrong in the existing code ? Other than it being very complex. How did its calculations go wrong ?
>
> PS if I just use simple ASCII text and set Node Orientation to RTL, then with your fix
> then left arrow moves right and right arrow moves left.
> In JDK 8 left arrow moved left and right arrow moved right.
> With an FX 17 build I have around, then the nothing moves.
> So it looks like things might have been better when this code was written and some other change broke it.
> Your fix unbreaks it, but I'm not entirely sure its restoring intended behaviour.

what if you have a mixture of RTL and LTR text in JDK8?
I am going to double check the behavior in JTextArea as well.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1220#discussion_r1350861969