<Swing Dev> [11]RFR:JDK-8208640: [a11y][macosx] Unable to navigate between Radiobuttons in Radio group using keyboard

[Phil Race]

On 08/13/2018 12:08 AM, Krishna Addepalli wrote:
>
> Hi Phil,
>
> I have run the JCK tests for JRadioButton and also focus traversal, 
> and found no failures.
>
> Here is the new webrev with additional formatting issues fixed: 
> http://cr.openjdk.java.net/~kaddepalli/8208640/webrev02/ 
> <http://cr.openjdk.java.net/%7Ekaddepalli/8208640/webrev02/>
>

You are referring to this comment of mine ?

 > You may have fixed the "){" pattern but I still see "if(" in v.01.

But v02 still has these

+ if(!isValidRadioButtonObj(eventSrc))
+ if(!isValidRadioButtonObj(curElement)) + if(activeBtn != null) { 
Comparing time stamps it looks like you just reposted v01 as v02 
http://cr.openjdk.java.net/~kaddepalli/8208640/webrev01/ 
http://cr.openjdk.java.net/~kaddepalli/8208640/webrev02/ I am APPROVING 
the fix, and assuming that this will be taken care of in the push. Go 
ahead and file the jdk11-fix-request paperwork ASAP. -phil.

> Thanks,
>
> Krishna
>
> *From:*Prasanta Sadhukhan
> *Sent:* Monday, August 13, 2018 9:00 AM
> *To:* Philip Race <philip.race at oracle.com>
> *Cc:* Krishna Addepalli <krishna.addepalli at oracle.com>; 
> swing-dev at openjdk.java.net
> *Subject:* Re: <Swing Dev> [11]RFR:JDK-8208640: [a11y][macosx] Unable 
> to navigate between Radiobuttons in Radio group using keyboard
>
> On 8/12/2018 11:02 PM, Philip Race wrote:
>
>     I did notice the inner classes are not static, but this is the same as
>     the class from which they were copied and it seems like they need
>     access to the enclosing instance.
>
>     Can you expand on the path to the problem you see ?
>
>     I don't think you will have a memory leak due to multiple calls to
>     installListeners()
>     which is how I read your statement. That would suggest you are
>     talking about
>     the SelectNextBtn instance leaking ?
>
>     First, the reference is one-way. Once the map entry is
>     over-written, the previous
>     SelectNextBtn() instance will become eligible to be gc'd since
>     there will be nothing
>     referencing it. Marginally inefficient, but that is all I see.
>
>     But what about just the single call to installListeners() ? And if
>     you mean the RadioButtonUI will
>     leak ? Multiple calls to installListeners() won't affect that so
>     far as I can see.
>
> I was referring to AquaRadioButtonUI leak. I guess you are right, if 
> uninstallListeners is called, then it can be gced.
>
> Regards
> Prasanta
>
>     So long as uninstallListeners() is called then the map entry will
>     be cleared,
>     making the values referenced eligible for GC, in turn making the
>     RadioButtonUI
>     eligible for GC - which probably will typically happen only if the
>     app switches L&F so that is rare.
>
>     So I think there is only a bug if uninstallListeners() is not
>     called, and I presume
>     there has to be a pattern to make sure this happens across Swing ...
>
>     -phil.
>
>     On 8/12/18, 9:13 AM, Prasanta Sadhukhan wrote:
>
>         One thing from 1st look...
>
>         Every time installListeners is called, it instantiates
>         SelectPreviousBtn and SelectNextBtn which is a inner class
>         (and not a static class) and it has a strong reference to
>         AquaButtonRadioUI so it may not be garbage collected and can
>         cause a leak.
>
>         Regards
>         Prasanta
>
>         On 8/11/2018 8:51 PM, Krishna Addepalli wrote:
>
>             Hi All,
>
>             Please review a fix for JDK-8208640:
>             https://bugs.openjdk.java.net/browse/JDK-8208640
>
>             Webrev:
>             http://cr.openjdk.java.net/~kaddepalli/8208640/webrev00/
>             <http://cr.openjdk.java.net/%7Ekaddepalli/8208640/webrev00/>
>
>             The problem is that the arrow key navigation for Aqua L&F
>             was not implemented, which is why neither the tab keys nor
>             the arrow keys were allowing to navigate through the radio
>             buttons through a button group.
>
>             Proposed fix is to copy the implementation from Basic L&F
>             into Aqua L&F. Also fixed the test that was written to
>             include the Aqua L&F to be tested, and it passes on Mac
>             successfully.
>
>             Thanks,
>
>             Krishna
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180813/c36629af/attachment.html>