<Swing Dev> [9] Review request for 8033699: Incorrect radio button behavior
Alexander Zvegintsev
alexander.zvegintsev at oracle.com
Thu Oct 9 09:52:49 UTC 2014
Hello Vivi
We can move Hashset creation to btnsInGroup's declaration, thus the
following lines can be removed
460 if (btnsInGroup == null)
461 btnsInGroup = new HashSet<JRadioButton>();
Otherwise the fix looks good to me.
Thanks,
Alexander.
On 10/09/2014 09:53 AM, Vivi An wrote:
> 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/20141009/ba938b87/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/20141009/ba938b87/attachment.png>
More information about the swing-dev
mailing list