RFR: 8303478: DatePicker throws uncatchable exception on tab out from garbled text [v4]

[Marius Hanl]

On Wed, 15 Nov 2023 09:37:23 GMT, brunesto <duke at openjdk.org> wrote:

>> The fix prevents the DatePicker from losing focus if the date is not parsable.
>
> brunesto has updated the pull request incrementally with one additional commit since the last revision:
> 
>   inlined

Looks good to me, just some minor things.
I also dug into the code and found out how this worked in JavaFX 8:


...
try {
    value = c.fromString(text);
} catch (Exception ex) {
    // Most likely a parsing error, such as DateTimeParseException
}
...


So this approach looks correct to me!
I just wonder if we want to catch `Exception` here as well, as it was done in the old code.
The contract then would be: If a converter throws any exception, we cancel the edit.
@andy-goryachev-oracle any objections here from your side?

modules/javafx.controls/src/test/java/test/javafx/scene/control/DatePickerTest.java line 746:

> 744:     @Test
> 745:     public void testFocusLostWithTypo() {
> 746:         //datePicker.setEditable(true); // other test cases have it .is this necessary?

Looks like you can remove this

modules/javafx.controls/src/test/java/test/javafx/scene/control/DatePickerTest.java line 753:

> 751:         // initial value
> 752:         datePicker.setValue(LocalDate.of(2015, 03, 25));
> 753:         assertEquals("3/25/2015",datePicker.getEditor().getText());

Minor: A space after comma is missing

modules/javafx.controls/src/test/java/test/javafx/scene/control/DatePickerTest.java line 756:

> 754: 
> 755:         // set misformatted text
> 756:         //stageLoader.getStage().requestFocus(); // other test cases have it . is this necessary?

same here, remove as it works without :)

modules/javafx.controls/src/test/java/test/javafx/scene/control/DatePickerTest.java line 764:

> 762: 
> 763:         // check that value remains unchanged, and text is reverted
> 764:         assertEquals(LocalDate.of(2015, 03, 25),datePicker.getValue());

Minor: A space after comma is missing

modules/javafx.controls/src/test/java/test/javafx/scene/control/DatePickerTest.java line 765:

> 763:         // check that value remains unchanged, and text is reverted
> 764:         assertEquals(LocalDate.of(2015, 03, 25),datePicker.getValue());
> 765:         assertEquals("3/25/2015",datePicker.getEditor().getText());

Minor: A space after comma is missing

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

PR Review: https://git.openjdk.org/jfx/pull/1274#pullrequestreview-1732643564
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1394584158
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1394585258
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1394584665
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1394585403
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1394585502