RFR: 8342462: TextAreaSkin: remove USE_MULTIPLE_NODES

[Kevin Rushforth]

On Wed, 16 Oct 2024 23:01:39 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> Removed "not yet fully implemented" USE_MULTIPLE_NODES and related code.
> 
> I would like to remove the early unfinished idea of using multiple Text nodes in TextAreaSkin to clean up the code, to make it easier to do fixes for [JDK-8342233](https://bugs.openjdk.org/browse/JDK-8342233) and [JDK-8296266](https://bugs.openjdk.org/browse/JDK-8296266).
> 
> Also some minor cleanup.
> 
> ## Summary of Changes
> 
> - removed USE_MULTIPLE_NODES and code paths that correspond to its `true` value
> - permanently adding one Text node to `paragraphNodes` Group, keeping the latter for compatibility purposes
> - removed any code that scans `paragraphNodes` children
> - using `getTextNode()` in place of `((Text)paragraphNodes.getChildren().get(0))`

This looks equivalent to the current code, and I couldn't see anything wrong in my testing. I left a couple questions inline.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextAreaSkin.java line 729:

> 727:     @Override
> 728:     protected PathElement[] getUnderlineShape(int start, int end) {
> 729:         return getTextNode().underlineShape(start, end);

The former code would return `null` if `start` > `textNode.textProperty().getValueSafe().length()`. I think this is OK, given that the implementation of `Text::underlineShape` checks -- what do you think? This applies to `getRangeShape`, too.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextAreaSkin.java line 821:

> 819:         if (y < contentView.snappedTopInset()) {
> 820:             // Select the character at x in the first row
> 821:             return 0;

I see that the implementation of the now-removed `getNextInsertionPoint` method also returned `0` unconditionally, so this is equivalent, but... isn't the comment wrong, since `x` isn't used in this case?

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextAreaSkin.java line 824:

> 822:         } else if (y > contentView.snappedTopInset() + contentView.getHeight()) {
> 823:             // Select the character at x in the last row
> 824:             return (textArea.getLength() - paragraphNode.getText().length());

Same question about the comment being wrong.

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1601#pullrequestreview-2382994268
PR Review Comment: https://git.openjdk.org/jfx/pull/1601#discussion_r1809276896
PR Review Comment: https://git.openjdk.org/jfx/pull/1601#discussion_r1809316134
PR Review Comment: https://git.openjdk.org/jfx/pull/1601#discussion_r1809324143