RFR: 8300084: AquaFileChooserUI.getDefaultButton returns null [v2]
Tejesh R
tr at openjdk.org
Mon Jan 16 13:49:26 UTC 2023
On Mon, 16 Jan 2023 13:33:04 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
> > > This case is entirely covered by the test in #11901 for [JDK-8299522](https://bugs.openjdk.org/browse/JDK-8299522), which also ensures the button text isn't empty.
> > > Adding 8300084 to the `@bug` tag in that test is enough. It was my [original suggestion](https://github.com/openjdk/jdk/pull/11901#discussion_r1068659255):
> > > > Now you can fix this bug so that Aqua L&F doesn't return null and add that bugid to this test.
> >
> >
> > Yeah, but since #11901 handles only Approve button size except for Aqua L&F and this fix is only for Aqua L&F I thought it would be better to not to maintain any dependency between the test/bug (Though this bug was initiated as part of #11901 fix). I hope maintaining simple test case for two bug would be fine.
>
> It's exactly the point: **include Aqua L&F in #11901**. The Aqua L&F is excluded only because `getDefaultButton` returns null, isn't it? This fix makes it return non-null, therefore Aqua L&F can be included in testing.
>
> This fix should be integrated before [JDK-8299522](https://bugs.openjdk.org/browse/JDK-8299522). I will not approve #11901 until Aqua L&F is part of the `CustomApproveButtonTest.java` test.
The Aqua L&F is excluded not only because `getDefaultButton` returns null, but it is also excluded because the fix doesn't apply to Aqua L&F. Still if u insist to use the same test then can do that, as u said first this fix will go in and then #11901.
> test/jdk/javax/swing/JFileChooser/AquaDefaultButtonTest.java line 46:
>
>> 44: JFileChooser fc = new JFileChooser();
>> 45: JButton defApproveBtn = fc.getUI().getDefaultButton(fc);
>> 46: if (defApproveBtn == null) {
>
> Suggestion:
>
> JButton approve = fc.getUI().getDefaultButton(fc);
> if (approve == null) {
>
> On the scope of two lines, its type — `JButton` — as well as method name makes it obvious it's a _button_.
>
> `approveButton` would also be good.
>
> Yet I am for removing this test completely because it's functionality is entirely covered by the `CustomApproveButtonTest.java` in #11901.
Ok, then I'll remove this test and update https://github.com/openjdk/jdk/pull/11901.
-------------
PR: https://git.openjdk.org/jdk/pull/12008
More information about the client-libs-dev
mailing list