RFR: 8286552: TextFormatter: UpdateValue/UpdateText is called, when no ValueConverter is set

[Kevin Rushforth]

On Tue, 10 May 2022 20:03:08 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> A common reason for using the `TextFormatter` is the need for a `filter`.
> Therefore, the following constructor is typically used:
> `public TextFormatter(@NamedArg("filter") UnaryOperator<Change> filter) { ... }`
> 
> With that, no `valueConverter` is set in the `TextFormatter`.
> 
> When a `TextField` will commit his value, `TextFormatter.updateValue(...)` is called.
> Since `valueConverter` is null, an NPE is thrown and catched inside it and `TextFormatter.updateText()` is called  (which will call `TextInputControl.updateText(...)`), which won't do anything either since `valueConverter` is still not set (=null).
> 
> A minor improvement here is to just skip `TextFormatter.updateValue(...)` and therefore `TextFormatter.updateText()` (-> `TextInputControl.updateText(...)`), since this methods won't do anything without a `valueConverter`.
> With that change, no exception (NPE) is thrown and catched, and therefore also exception breakpoints are not called (can be annoying ;)).
> This will also slightly increase the performance, as throwing exceptions is a (minor) bottleneck.
> 
> Added tests for all `TextFormatter` functionalities, which pass before and after.

The fix and tests look good, with a couple requested changes.

modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java line 40:

> 38:  * <ul>
> 39:  *     <li>A filter ({@link #getFilter()}) that can intercept and modify user input. This helps to keep the text
> 40:  *     in the desired format. A default text supplier can be used to provide the initial text.</li>

I know this is a simple typo, but it is unrelated to your bug fix, and is in public API docs, so I'd like to see it go in separately under a "Fix mistakes in docs" bug. I filed [JDK-8286678](https://bugs.openjdk.java.net/browse/JDK-8286678) to track this and any other such issues that arise (as we've done for most recent releases).

modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java line 202:

> 200: 
> 201:     void updateValue(String text) {
> 202:         if (valueConverter != null &&!value.isBound()) {

Minor: please add a space between the `&&` and `!` operators.

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

PR: https://git.openjdk.java.net/jfx/pull/794