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

Andy Goryachev angorya at openjdk.org
Thu Nov 9 17:52:33 UTC 2023


On Tue, 31 Oct 2023 19:20:48 GMT, brunesto <duke at openjdk.org> wrote:

> The fix prevents the DatePicker from losing focus if the date is not parsable.

Just preliminary comments, will do a full review once OCA process is completed.

tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.

it's a brand new file, I wonder if it should have 2023 only

tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java line 36:

> 34: import org.junit.Before;
> 35: import org.junit.BeforeClass;
> 36: import org.junit.Test;

could we switch to junit5 please?

tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java line 98:

> 96:     // 3. Click on button to grab the focus and hence attempt to datePicker.commitValue()
> 97:     // 4. Verify that in case of typo, the value was reverted, as well as the editor's text
> 98:     public void testDatePickerCommit(boolean typo) throws Exception {

thank you for explaining what the test cases attempt to do!

tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java line 141:

> 139:         // 6 check if onChangeListener was called (already working as expected before the patch)
> 140:         if (typo)
> 141:             Assert.assertEquals(onChangeListenerCalled, 0);

please always use { } in `if` statements

tests/system/src/test/java/test/robot/javafx/scene/DatePickerOnFocusLostTest.java line 185:

> 183:             button = new Button("...");
> 184:             root.getChildren().add(button);
> 185: 

extra newline (here and elsewhere)

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

PR Review: https://git.openjdk.org/jfx/pull/1274#pullrequestreview-1708442005
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378943077
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378939425
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378940060
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378938085
PR Review Comment: https://git.openjdk.org/jfx/pull/1274#discussion_r1378938680


More information about the openjfx-dev mailing list