<Swing Dev> [9] Review request for 8033699: Incorrect radio button behavior

Vivi An vivi.an at oracle.com
Thu Oct 9 05:53:43 UTC 2014


Thank you Alexander,

New patch with fix:
http://cr.openjdk.java.net/~van/8033699/webrev.08/

Regards,

Vivi

On 10/8/2014 11:36 AM, Alexander Zvegintsev wrote:
> Hello Vivi,
>
> getFocusTransferBaseComponent():
>              Window parentWnd = SwingUtilities.getWindowAncestor((Component) activeBtn);
>              if (parentWnd instanceof Container) {
>                  Container container = (Container)parentWnd;
> Window extending Container class, hence there is no need to use 
> instanceof, null
> check is enough here. container variable is redundant here, you may 
> reuse parentWnd.
> There are many redundant class casts in this function.
> Also I suggest to use ternary operator to increase readability.
>
> Something like this:
>
>         Component getFocusTransferBaseComponent(boolean next) {
>             Component focusBaseComp = activeBtn;
>             Window container = 
> SwingUtilities.getWindowAncestor(activeBtn);
>             if (container != null) {
>                 FocusTraversalPolicy policy = 
> container.getFocusTraversalPolicy();
>                 Component comp = next
>                         ? policy.getComponentAfter(container, activeBtn)
>                         : policy.getComponentBefore(container, activeBtn);
>
>                 // If next component in the button group, use 
> last/first button as base focus
>                 // otherwise, use the activeButton as the base focus
>                 if (containsInGroup(comp)) {
>                     focusBaseComp = next ? lastBtn : firstBtn;
>                 }
>             }
>
>             return focusBaseComp;
>         }
>
> Small typo in test: Seperate - > Separate (2 times)
>
> --
> Thanks,
> Alexander.
> On 10/08/2014 09:45 AM, Vivi An wrote:
>> Hello Alexander,
>>
>> Latest patched is ready for review, test case is updated too:
>> http://cr.openjdk.java.net/~van/8033699/webrev.07/
>>
>> Thanks
>>
>> Vivi
>>
>> On 10/2/2014 2:46 PM, Vivi An wrote:
>>> Thanks Alexander. Will look into it.
>>>
>>> Regards,
>>>
>>> ~ Vivi
>>>
>>> On 10/2/2014 12:20 AM, Alexander Zvegintsev wrote:
>>>> Sorry, I wasn't clear,
>>>> provided code with the "MIDDLE" button is just an example what user 
>>>> can write (pretty odd one btw)
>>>> it is not in your tests.
>>>>
>>>> By lost functionality I mean that after this fix this user can't 
>>>> navigate to the "MIDDLE" button via keyboard
>>>> (or to the btnEnd after ButtonGroup reordering).
>>>>
>>>> --
>>>> Thanks,
>>>> Alexander.
>>>> 02.10.2014 0:40, Vivi An wrote:
>>>>> Thanks Alexander.
>>>>>
>>>>> I have some questions,  please see comments inline.
>>>>>
>>>>> Regards,
>>>>>
>>>>> ~ Vivi
>>>>>
>>>>> On 10/1/2014 5:47 AM, Alexander Zvegintsev wrote:
>>>>>> Hello Vivi,
>>>>>>
>>>>>> the fix looks good to me in general, but we lost some functionality,
>>>>> By functionality,  you mean the functionality of the test, am I right?
>>>>>
>>>>>> here is some modified code from the previous version of the test:
>>>>>>
>>>>>>         ButtonGroup btnGrp = new ButtonGroup();
>>>>>>         btnGrp.add(radioBtn1);
>>>>>>         btnGrp.add(radioBtn2);
>>>>>>         btnGrp.add(radioBtn3);
>>>>>>         radioBtn1.setSelected(true);
>>>>>>
>>>>>>         mainFrame.getContentPane().add(btnStart);
>>>>>>         mainFrame.getContentPane().add(radioBtn1);
>>>>>>         mainFrame.getContentPane().add(radioBtn2);
>>>>>>         mainFrame.getContentPane().add(new JButton("MIDDLE"));
>>>>>>         mainFrame.getContentPane().add(radioBtn3);
>>>>>>         mainFrame.getContentPane().add(btnEnd);
>>>>> Just double checked the test in previous version and current 
>>>>> version, did not find a button "MIDDLE" like previous code 
>>>>> snippet,  or I misunderstood, this is pseudo code?
>>>>> Compare to the old test code, new changes made are:
>>>>> 1) Radio button group( radioBtn1, radioBtn2 and radioBtn1 ) added 
>>>>> to a panel ("box" in below code snippet) and then to main frame,  
>>>>> border also added too to separate from new added non grouped 
>>>>> single radio button
>>>>> 2) A single radio button (Not Grouped) is added to cover the bug 
>>>>> found in last round
>>>>> 3) Tests 1- 6 are adjusted for preceding change to cover areas 
>>>>> would like to test
>>>>>
>>>>> Below is snippet from latest version, the "box" is the panel which 
>>>>> contains radio button group, a screenshot added too
>>>>>   110         mainFrame.getContentPane().add(btnStart);
>>>>>   111         mainFrame.getContentPane().add(box);
>>>>>   112         mainFrame.getContentPane().add(radioBtnSingle);
>>>>>   113         mainFrame.getContentPane().add(btnEnd);
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> After this fix we are not able to select "MIDDLE" button with a 
>>>>>> keyboard anymore.
>>>>>> If we change ButtonGroup order to
>>>>>>
>>>>>>         btnGrp.add(radioBtn3);
>>>>>>         btnGrp.add(radioBtn2);
>>>>>>
>>>>>> We get into a loop and we are unable to select btnEnd and all 
>>>>>> following components(Shift+Tab can help in that case).
>>>>>> I think that these cases deserves some attention.
>>>>>>
>>>>>> -- 
>>>>>> Thanks,
>>>>>> Alexander.
>>>>>>
>>>>>> On 10/01/2014 01:15 AM, Vivi An wrote:
>>>>>>> Hello Alexander,
>>>>>>>
>>>>>>> Thanks for the review.
>>>>>>>
>>>>>>> New patch is available under 
>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.06/
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Vivi
>>>>>>>
>>>>>>> On 9/19/2014 3:22 AM, Alexander Zvegintsev wrote:
>>>>>>>> Hello Vivi,
>>>>>>>>
>>>>>>>> Since you are touching this code, could you please add missing 
>>>>>>>> @Override annotations?
>>>>>>>>
>>>>>>> Added
>>>>>>>> void jumpToNextComponent(boolean next) {
>>>>>>>>             if (!getButtonGroupInfo())
>>>>>>>>                 return;
>>>>>>>>
>>>>>>>> This leads to an issue: we can't switch to a next component 
>>>>>>>> with a TAB key if JRadioButton
>>>>>>>> is not in a ButtonGroup. (It may be a reason to write another 
>>>>>>>> test.)
>>>>>>> Fixed
>>>>>>>>
>>>>>>>>     private boolean isValidRadioButtonObj(Object obj) {
>>>>>>>>         return ((obj != null) && (obj instanceof JRadioButton) &&
>>>>>>>>                     ((JRadioButton) obj).isVisible() &&
>>>>>>>>                     ((JRadioButton) obj).isEnabled());
>>>>>>>>     }
>>>>>>> Fixed
>>>>>>>>
>>>>>>>> There is no need to do a null check before instanceof [1]
>>>>>>>>
>>>>>>>>      * @param evt, the event object.
>>>>>>>>      * @param next, indicate if it's next one
>>>>>>>>      */
>>>>>>>>     private void selectRadioButton(ActionEvent event, boolean 
>>>>>>>> next) {
>>>>>>>>
>>>>>>>> typo: evt should be event
>>>>>>>>
>>>>>>> Fixed
>>>>>>>>
>>>>>>>> I run the test and it fails at least on Linux. It happens due 
>>>>>>>> to a big auto delay:
>>>>>>>> Robot holds key for too long so system sends TAB key multiple 
>>>>>>>> time.
>>>>>>>> So I think auto delay should be about 100 ms.
>>>>>>>>
>>>>>>>> robot.setAutoDelay(300);
>>>>>>>>
>>>>>>>> It is enough to call it once after Robot creation, this will 
>>>>>>>> automatically add a 300 ms delay
>>>>>>>> between Robot generated events (such as keyPress, mouseMove, etc).
>>>>>>>> If you really want a delay between test runs please use 
>>>>>>>> Thread.sleep().
>>>>>>>>
>>>>>>>> I see many calls to hitKey() and then to realSync(), so we can 
>>>>>>>> move realSync() call to the end of hitKey().
>>>>>>>> checkResult() may be combined with runTest(), hence we call one 
>>>>>>>> function instead of two.
>>>>>>>>
>>>>>>>>         int sum = 0;
>>>>>>>>         for (int i = 0; i < testResults.length; i++) {
>>>>>>>>             if (testResults[i] == false)
>>>>>>>>                 sum++;
>>>>>>>>         }
>>>>>>>>
>>>>>>>>         if (sum != 0){
>>>>>>>>             System.out.println("Total 5 tests, failed test(s): 
>>>>>>>> " + sum);
>>>>>>>>             throw new RuntimeException("Test failed.");
>>>>>>>>         }
>>>>>>>>
>>>>>>>>
>>>>>>>> I don't think that this logic is applicable here, each test 
>>>>>>>> depends on previous: Test2 failure means that
>>>>>>>> focus is on a wrong component, so all following tests will fail.
>>>>>>>> BTW, this code is unreachable in case of test failure, since 
>>>>>>>> checkResult() already throws an exception.
>>>>>>>>
>>>>>>>> http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.20.2 
>>>>>>>>
>>>>>>> Test is updated, non-grouped radio button test is added,  test 
>>>>>>> passed on Oracle Linux 5 u6 32 bits and Windows 7.
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Thanks,
>>>>>>>> Alexander.
>>>>>>>>
>>>>>>>> On 09/18/2014 09:23 PM, Vivi An wrote:
>>>>>>>>> Thank you Alexandr.
>>>>>>>>>
>>>>>>>>> Need one more review for this. Alexander, could you please 
>>>>>>>>> take a look at below fix?
>>>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.05/
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>>
>>>>>>>>> Vivi
>>>>>>>>>
>>>>>>>>> On 9/18/2014 12:55 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>
>>>>>>>>>>   The fix looks good to me.
>>>>>>>>>>
>>>>>>>>>>   Thanks,
>>>>>>>>>>   Alexandr.
>>>>>>>>>>
>>>>>>>>>> On 9/18/2014 3:09 AM, Vivi An wrote:
>>>>>>>>>>> Hello Alex,
>>>>>>>>>>>
>>>>>>>>>>> On 9/17/2014 12:44 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>> On 9/17/2014 9:17 AM, Vivi An wrote:
>>>>>>>>>>>>> You are right. Fixed now.
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.04/
>>>>>>>>>>>>
>>>>>>>>>>>>  528             if (e.isShiftDown()) {
>>>>>>>>>>>>  529                 if (e.getKeyCode() == KeyEvent.VK_TAB) {
>>>>>>>>>>>>  537                         // ... 
>>>>>>>>>>>> btnGroupInfo.jumpToNextComponent(false);
>>>>>>>>>>>>  539                 }
>>>>>>>>>>>>  540             } else if (e.getKeyCode() == 
>>>>>>>>>>>> KeyEvent.VK_TAB) {
>>>>>>>>>>>>  548                     // ... 
>>>>>>>>>>>> btnGroupInfo.jumpToNextComponent(true);
>>>>>>>>>>>>  550             }
>>>>>>>>>>>>
>>>>>>>>>>>>   The code in the if/else branches is almost the same 
>>>>>>>>>>>> except the true/false argument.
>>>>>>>>>>>>   I would suggest to unify them in the following way:
>>>>>>>>>>>>
>>>>>>>>>>>>      if (e.getKeyCode() == KeyEvent.VK_TAB) {
>>>>>>>>>>>>         // ... jumpToNextComponent(!e.isShiftDown())
>>>>>>>>>>>>      }
>>>>>>>>>>>>
>>>>>>>>>>> Fixed.
>>>>>>>>>>>>
>>>>>>>>>>>>  510             if (next)
>>>>>>>>>>>>  511                 KeyboardFocusManager.
>>>>>>>>>>>>  512 
>>>>>>>>>>>> getCurrentKeyboardFocusManager().focusNextComponent((JComponent)lastBtn);
>>>>>>>>>>>>  513             else
>>>>>>>>>>>>  514                 KeyboardFocusManager.
>>>>>>>>>>>>  515 
>>>>>>>>>>>> getCurrentKeyboardFocusManager().focusPreviousComponent((JComponent)firstBtn);
>>>>>>>>>>>>
>>>>>>>>>>>>  This code can be also a little bit optimized:
>>>>>>>>>>>>
>>>>>>>>>>>> KeyboardFocusManager.getCurrentKeyboardFocusManager().
>>>>>>>>>>>>         focusPreviousComponent((JComponent) (next ? lastBtn 
>>>>>>>>>>>> : firstBtn));
>>>>>>>>>>>>
>>>>>>>>>>> It's different function call,  one for focusNextComponent 
>>>>>>>>>>> and the other for focusPreviousComponent, not able to 
>>>>>>>>>>> optimize in suggested way.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.05/
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>>
>>>>>>>>>>> Vivi
>>>>>>>>>>>
>>>>>>>>>>>>  Thanks,
>>>>>>>>>>>>  Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you
>>>>>>>>>>>>>
>>>>>>>>>>>>> ~ Vivi
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/16/2014 12:39 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>   I have missed one more case:
>>>>>>>>>>>>>>  527         public void keyPressed(KeyEvent e) {
>>>>>>>>>>>>>>  528             if (e.getKeyCode() == KeyEvent.VK_TAB) {
>>>>>>>>>>>>>>                         // jumpToNextComponent(true)
>>>>>>>>>>>>>>  538             }
>>>>>>>>>>>>>>  539
>>>>>>>>>>>>>>  540             if (e.getKeyCode() == KeyEvent.VK_TAB && 
>>>>>>>>>>>>>> e.isShiftDown()) {
>>>>>>>>>>>>>> //jumpToNextComponent(false)
>>>>>>>>>>>>>>  550             }
>>>>>>>>>>>>>>  551         }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>   It seems that if e.isShiftDown() is true then both 
>>>>>>>>>>>>>> jumpToNextComponent(true) and jumpToNextComponent(false) 
>>>>>>>>>>>>>> methods can be called.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>   Thanks,
>>>>>>>>>>>>>>   Alexandr.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 9/16/2014 7:38 AM, Vivi An wrote:
>>>>>>>>>>>>>>> Fixed. New Webrev: 
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.03/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks Alex.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> ~ Vivi
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 9/15/2014 2:59 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>> On 9/12/2014 8:56 PM, Vivi An wrote:
>>>>>>>>>>>>>>>>> Thanks Alexander.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> All items addressed except last one, please see 
>>>>>>>>>>>>>>>>> comments inline.
>>>>>>>>>>>>>>>>> New Webrev: 
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~van/8033699/webrev.02/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>     163         if ( keyListener != null ) {
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>     Could you remove spaces in the brackets? After code 
>>>>>>>>>>>>>>>> formatting it should be "if (keyListener != null) {"
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 543 if (next && btnGroupInfo.lastBtn != null)
>>>>>>>>>>>>>>>>>>  544 KeyboardFocusManager.
>>>>>>>>>>>>>>>>>>  545 
>>>>>>>>>>>>>>>>>> getCurrentKeyboardFocusManager().focusNextComponent((JComponent)btnGroupInfo.lastBtn);
>>>>>>>>>>>>>>>>>>  546             else if (btnGroupInfo.firstBtn != null)
>>>>>>>>>>>>>>>>>>  547 KeyboardFocusManager.
>>>>>>>>>>>>>>>>>>  548 
>>>>>>>>>>>>>>>>>> getCurrentKeyboardFocusManager().focusPreviousComponent((JComponent)btnGroupInfo.firstBtn);
>>>>>>>>>>>>>>>>>>  549         }
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Should the first button be focused If next is true 
>>>>>>>>>>>>>>>>>> and last button is null?
>>>>>>>>>>>>>>>>> Don't think last button could be null when this 
>>>>>>>>>>>>>>>>> function is triggered, last and first will always be 
>>>>>>>>>>>>>>>>> set in case there is at least one valid (enabled and 
>>>>>>>>>>>>>>>>> visible) radio button in the group.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>   It seems that lastBtn != null and firstBtn != null 
>>>>>>>>>>>>>>>> checks are not necessary in this case.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>   Thanks,
>>>>>>>>>>>>>>>>   Alexandr.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> ~ Vivi
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 9/8/2014 5:43 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>>>>> On 9/4/2014 10:56 PM, Vivi An wrote:
>>>>>>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Please review fix for JDK-8033699. Changes made:
>>>>>>>>>>>>>>>>>>>>> 1) When tab or shift-tab key pressed, focus moved 
>>>>>>>>>>>>>>>>>>>>> to next/previous
>>>>>>>>>>>>>>>>>>>>> component outside of the radio button group
>>>>>>>>>>>>>>>>>>>>> 2) Using up/down or left/right arrow key to change 
>>>>>>>>>>>>>>>>>>>>> selection inside
>>>>>>>>>>>>>>>>>>>>> radio button group
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8033699
>>>>>>>>>>>>>>>>>>>>> Webrev:http://cr.openjdk.java.net/~van/8033699/webrev.00/ 
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>   56     private KeyListener keyListener = null;
>>>>>>>>>>>>>>>>>>>>   57     private KeyHandler handler = null;
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> It seems that these both fields point to the same 
>>>>>>>>>>>>>>>>>>>> object. Is it possible to get rid of one of them?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  152         if ( keyListener != null ) {
>>>>>>>>>>>>>>>>>>>>  153 button.removeKeyListener( keyListener );
>>>>>>>>>>>>>>>>>>>>  154         }
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Some UIs also set the listener to null in the 
>>>>>>>>>>>>>>>>>>>> uninstallUI()/uninstallListeners() methods (like 
>>>>>>>>>>>>>>>>>>>> BasicComboBoxUI or BasicColorChooserUI).
>>>>>>>>>>>>>>>>>>>> Should the keyListener also be set to null to free 
>>>>>>>>>>>>>>>>>>>> the KeyHandler object?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  369         if (!(eventSrc instanceof JRadioButton &&
>>>>>>>>>>>>>>>>>>>>  370 ((JRadioButton) eventSrc).isVisible() && 
>>>>>>>>>>>>>>>>>>>> ((JRadioButton) eventSrc).isEnabled()))
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> This check is used several times. It can be moved 
>>>>>>>>>>>>>>>>>>>> to a separate method.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  373         // Get the button model from the source.
>>>>>>>>>>>>>>>>>>>>  374         ButtonModel model = ((AbstractButton) 
>>>>>>>>>>>>>>>>>>>> eventSrc).getModel();
>>>>>>>>>>>>>>>>>>>>  375         if (!(model instanceof 
>>>>>>>>>>>>>>>>>>>> DefaultButtonModel))
>>>>>>>>>>>>>>>>>>>>  376             return;
>>>>>>>>>>>>>>>>>>>>  377
>>>>>>>>>>>>>>>>>>>>  378         // If the button model is 
>>>>>>>>>>>>>>>>>>>> DefaultButtonModel, and use it, otherwise return.
>>>>>>>>>>>>>>>>>>>>  379 DefaultButtonModel bm = (DefaultButtonModel) 
>>>>>>>>>>>>>>>>>>>> model;
>>>>>>>>>>>>>>>>>>>>  380         if (bm == null)
>>>>>>>>>>>>>>>>>>>>  381             return;
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The second check is not necessary because (model 
>>>>>>>>>>>>>>>>>>>> instanceof DefaultButtonModel) returns false for 
>>>>>>>>>>>>>>>>>>>> null model.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  404         AbstractButton curElement = null;
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>   The curElement variable declaration could be 
>>>>>>>>>>>>>>>>>>>> moved into the 'while' block.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  408             if (curElement instanceof 
>>>>>>>>>>>>>>>>>>>> JRadioButton &&
>>>>>>>>>>>>>>>>>>>>  409 ((JRadioButton) curElement).isVisible() &&
>>>>>>>>>>>>>>>>>>>>  410 ((JRadioButton) curElement).isEnabled()){
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>   It is possible to use 'continue' here to not put 
>>>>>>>>>>>>>>>>>>>> the other code inside the 'if' block.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  418                 else if (!srcFound){
>>>>>>>>>>>>>>>>>>>>  422                 } else if (srcFound && nextBtn 
>>>>>>>>>>>>>>>>>>>> == null){
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> It is not necessary to check the srcFound in the 
>>>>>>>>>>>>>>>>>>>> second 'if' because it should already have true value.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  444             if (newSelectedBtn != null){
>>>>>>>>>>>>>>>>>>>>  445 newSelectedBtn.requestFocusInWindow();
>>>>>>>>>>>>>>>>>>>>  446 newSelectedBtn.setSelected(true);
>>>>>>>>>>>>>>>>>>>>  447             }
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Is it possible here that newSelectedBtn == eventSrc?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  522         private void 
>>>>>>>>>>>>>>>>>>>> jumpToNextComponent(JRadioButton btn, boolean next){
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The code that retrieves elements from a button 
>>>>>>>>>>>>>>>>>>>> group is also used in the selectRadioButton()
>>>>>>>>>>>>>>>>>>>> and can be moved to a separate method.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> JPRT build succeeded, JCK tests for JRadiobutton 
>>>>>>>>>>>>>>>>>>>>> and JCheckBox all passed.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Thank you
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Vivi
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20141008/339a3114/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 32466 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20141008/339a3114/attachment.png>


More information about the swing-dev mailing list