Ok. I've approved it. Please push.
-phil.
On 8/14/18, 1:07 AM, Krishna Addepalli wrote:
Oops, my bad. I was trying to upload a webrev with rsync, and by
mistake wrong folder was selected. I have replaced
webrev02(http://cr.openjdk.java.net/~kaddepalli/8208640/webrev02/
<http://cr.openjdk.java.net/%7Ekaddepalli/8208640/webrev02/>), with
the correct upload, with all the formatting issues fixed.
I have also filed the “jdk11-fix-request” for the bug, and put in a
comment explaining the problem, fix and the tests that have been done.
Thanks,
Krishna.
*From:*Phil Race
*Sent:* Tuesday, August 14, 2018 12:43 AM
*To:* Krishna Addepalli <[email protected]>; Prasanta
Sadhukhan <[email protected]>
*Cc:* [email protected]
*Subject:* Re: <Swing Dev> [11]RFR:JDK-8208640: [a11y][macosx] Unable
to navigate between Radiobuttons in Radio group using keyboard
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/%7Ekaddepalli/8208640/webrev01/>
http://cr.openjdk.java.net/~kaddepalli/8208640/webrev02/
<http://cr.openjdk.java.net/%7Ekaddepalli/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 <[email protected]>
<mailto:[email protected]>
*Cc:* Krishna Addepalli <[email protected]>
<mailto:[email protected]>; [email protected]
<mailto:[email protected]>
*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
