Re: <Swing Dev> [9] Review request for 8165207: [macosx] Test javax/swing/Popup/TaskbarPositionTest.java fails on Mac 10.12

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review request for 8165207: [macosx] Test javax/swing/Popup/TaskbarPositionTest.java fails on Mac 10.12

Avik Niyogi
Hi Alexander,
The following is my input for this webrev:

In case the LAF is Aqua, the check for combobox alignment is skipped. 
Instead, if default LAF is Aqua, the LAF should be changed to cross platform LAF to check if it works on other LAF or not.
Either run this in a loop with checks for all LAF versions available on OS, or check for default LAF at the beginning and change it to Cross-Platform LAF to check for alignment.
The current changes makes the test pass but does seem to give false positives by process of elimination for Aqua LAF.

Also, found the following minor issues which could be looked into if possible:
variables done and error are unused.
numData, dayData and mnDayData could be declared as final.
@override annotations are missing for methods used in ComboPopupCheckListenerPopupHandler.
panel is redeclared within scope instead of reusing available variable in method createContentPane().
wildcard * is used in import statements which according to guidelines should be replace to use only the classes required.

With Regards,
Avik Niyogi
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review request for 8165207: [macosx] Test javax/swing/Popup/TaskbarPositionTest.java fails on Mac 10.12

alexander popov

Yep, its reasonable!

Will fix that and update diff.


On 1/11/2017 1:39 PM, Avik Niyogi wrote:
Hi Alexander,
The following is my input for this webrev:

In case the LAF is Aqua, the check for combobox alignment is skipped. 
Instead, if default LAF is Aqua, the LAF should be changed to cross platform LAF to check if it works on other LAF or not.
Either run this in a loop with checks for all LAF versions available on OS, or check for default LAF at the beginning and change it to Cross-Platform LAF to check for alignment.
The current changes makes the test pass but does seem to give false positives by process of elimination for Aqua LAF.

Also, found the following minor issues which could be looked into if possible:
variables done and error are unused.
numData, dayData and mnDayData could be declared as final.
@override annotations are missing for methods used in ComboPopupCheckListenerPopupHandler.
panel is redeclared within scope instead of reusing available variable in method createContentPane().
wildcard * is used in import statements which according to guidelines should be replace to use only the classes required.

With Regards,
Avik Niyogi

-- 
Best regards,
Alexander.
Loading...