On Mon, 11 Jan 2021 08:05:21 GMT, Prasanta Sadhukhan <psadhuk...@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. > > 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? I just rolled back the changes I did in this file in JDK-8249548. Looks like this was has been there for some time. See the file in commit made in this file before JDK-8249548 https://github.com/openjdk/jdk/blob/c18080fef714949221c8ddabc4e23d409575c3d3/src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java. But yes, it should not be done and I will remove it and update the PR. ------------- PR: https://git.openjdk.java.net/jdk16/pull/99
