On Sat, 9 Jan 2021 12:47:48 GMT, Pankaj Bansal <pban...@openjdk.org> wrote:
> Please review a fix for jdk16 > > Issue: In SwingSet2, if the user navigates through the demos, the demo gets > selected/starts on pressing left/right key only. There is no need to press > the "space" key. Earlier, on pressing the left/right key, only demo icon used > to get focused and user needed to press the "space" to actually select a demo. > > Cause: The SwingSet2 has JToggleButtons added to a ButtonGroup. Each > JToggleButton has an AbstractAction set on it, which loads the demo. Earlier, > when the user pressed Left/Right button, only the selected button used to > change. The Action set on JToggleButton used to perform only on pressing the > "Space" button. Now, the Action is performed on navigating the JToggleButtons > using Left/Right keys without the need to press the "space" key. > > This issue is a side effect of fix done for > https://bugs.openjdk.java.net/browse/JDK-8249548. The issues fixed in > JDK-8249548 were not present in JRadioButton as there was code available to > handle it in AquaButtonRadioUI and BasicRadioButtonUI. The fix done for > JDK-8249548 moved duplicate code from AquaButtonRadioUI and > BasicRadioButtonUI and moved it to BasicButtonUI, so this code is available > for JToggleButton and JRadioButton for all L&Fs. This was wrong as there is a > difference in behaviour for JRadioButtons added to ButtonGroup for AquaL&F > and other L&Fs. In AquaL&F, the AbstractAction set on JRadioButton is not > performed on button selection and user has to press "space". In other L&Fs, > the AbstractAction is performed on selection of Button itself without the > need to press "space". > > Fix: The current change fixes the issue in present bug and keeps the fixes > done in JDK-8249548. Following is the behaviour of JToggleButton and > JRadioButton for different L&Fs before JDK-8249548 fix, after JDK-8249548 fix > and after current fix. > > Before fix for JDK-8249548 > JToggleButton: > For all L&Fs, user can not navigate/select the buttons added to ButtonGroup > properly as mentioned in the JDK-8249548. > JRadioButton: > For Synth L&F (Nimbus L&F), user can not navigate/select the buttons added to > ButtonGroup. > For AquaL&F, user can select the buttons added to ButtonGroup by pressing > left/right key and needs to press the "space" to perform the set Action. > For Other L&Fs, user can select/navigate the buttons and the set Action is > also performed without pressing "space" > > After fix for JDK-8249548 > JToggleButton: > For all L&Fs, user can navigate/select the buttons added to ButtonGroups and > the AbstractAction set is performed without the need to press "space". > JRadioButton: > For all L&Fs, user can navigate/select the buttons added to ButtonGroups and > the AbstractAction set is performed without the need to press "space". > > After current fix: > JToggleButton: > For all L&Fs, user can navigate/select the buttons added to ButtonGroups. > User needs to press "space" to perform the set AbstractAction. > JRadioButton: > For AquaL&F, the behaviour before JDK-8249548 is restored, so user needs to > press the "space" to perform the set Action. > For all other L&Fs (including Synth L&F), the behaviour is same. User can > navigate/select the buttons added to ButtonGroups and set Action is performed > without pressing "space" > > I have run mach5 jobs with full jtreg/jck and all looks good. Links in JBS. > The test TestButtonGroupFocusTraversal.java is modified such that it fails > without current fix and passes after the fix. The fix also should be verified > by running SwingSet2. I guess you did not run jtreg job with latest problemlist as it did not run javax/swing/JRadioButton/ButtonGroupFocus/ButtonGroupFocusTest.java javax/swing/JRadioButton/8075609/bug8075609.java javax/swing/JRadioButton/8033699/bug8033699.java Please see this test pass with your change as some of them used to fail after JDK-8226892 which was reworked by fix of JDK-8249548 src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 228: > 226: if (listener != null) { > 227: b.removeMouseListener(listener); > 228: b.removeMouseListener(listener); Why are we removing same listener twice? ------------- PR: https://git.openjdk.java.net/jdk16/pull/99
