RFR: 8342462: TextAreaSkin: remove USE_MULTIPLE_NODES

Andy Goryachev angorya at openjdk.org
Mon Oct 21 19:24:16 UTC 2024


On Mon, 21 Oct 2024 18:16:01 GMT, Kevin Rushforth <kcr 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))`
>
> 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.

I think it's ok.  The javadoc for `TextInputControlSkin::underlineShape()` does not specify a null return value.

In fact, the javadoc for `Text::rangeShape()` and `::underlineShape()` say

`@return an array of {@code PathElement} which can be used to create a {@code Shape}`

implying that it should never be `null` as the return value is typically used in `Shape::getElements().setAll()` or `.addAll()` which will throw an NPE if the thing is null.  In fact, `Text::rangeShape()` (used for underline and range shape) always returns a non-null value.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1601#discussion_r1809385959


More information about the openjfx-dev mailing list