<Swing Dev> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the ResourceBundle for every call to toDisplayString

[Krishna Addepalli]

Hi Phil 

 

Thanks for the pointers. 

Since you mentioned that the bug pattern was observed before, I just did a quick search in accessibility module to see if any such cases are there. I could only find that for a Vector, which as you said is fine.

Second, I was also trying to understand if the "contains" function was really intended in the bug, but looking at the code, I did not find any reason that they should have used contains rather than "containsKey".

 

Sure will fix the space issue before pushing.

 

Thanks,

Krishna

 

From: Philip Race 
Sent: Tuesday, March 6, 2018 12:36 AM
To: Krishna Addepalli <krishna.addepalli at oracle.com>
Cc: swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the ResourceBundle for every call to toDisplayString

 

 

On 03/05/2018 05:39 AM, Krishna Addepalli wrote:

Hi Phil

 

Thank you for the review.

 

I have checked the accessibility package, and found that contains is used with a Vector of AccessibleState objects in the file AccessibleStateset.


contains is fine for a Vector .. it is the map case that folks can get wrong.




 

However in AccessibilityBundle, the table is used to store a set of values associated with a particular locale, and going by the context, I find little reason that using contains is intentional here. 


I did not suggest it was. I was observing that using contains instead of containsKey
is a mistake we've seen before. 




 

And regarding the testcase, the thought of making it headless crossed my mind while writing it, but I was not sure if just creating a Button is allowed in headless mode. 

Fortunately, I modified the testcase enough that, no swing widgets need to be created, and we can safely run the test in headless mode.


java.awt.Button is heavyweight and can't even be constructed in headless.
JButton is lightweight and can be constructed in headless mode.




 

Here is the new webrev: HYPERLINK "http://cr.openjdk.java.net/%7Ekaddepalli/8197785/webrev01/"http://cr.openjdk.java.net/~kaddepalli/8197785/webrev01/

  42     public static void main(String... args) throws Exception{

"Exception{" -> "Exception {"

Please fix before pushing.

Looks good other than that.


-phil.



 

Thanks,

Krishna

 

From: Philip Race 
Sent: Saturday, March 3, 2018 9:44 PM
To: Krishna Addepalli HYPERLINK "mailto:krishna.addepalli at oracle.com"<krishna.addepalli at oracle.com>
Cc: HYPERLINK "mailto:swing-dev at openjdk.java.net"swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the ResourceBundle for every call to toDisplayString

 

The fix is straightforward and I've seen this bug pattern before.
In fact there may even have been a sweep for uses of contains() to make sure it was as intended,
but if so it wasn't thorough enough.
But I'm wondering why the test extends Button and not JButton ?
I'm then further wondering if it could then be made headless .. ie no need to create or display a Frame ?

-phil.

On 3/3/18, 6:30 AM, Krishna Addepalli wrote: 

Hi All,

 

Please review a simple fix for JDK-8197785: https://bugs.openjdk.java.net/browse/JDK-8197785

Webrev:  HYPERLINK "http://cr.openjdk.java.net/%7Ekaddepalli/8197785/webrev00/"http://cr.openjdk.java.net/~kaddepalli/8197785/webrev00/

 

As the bug description suggests, AccessibleBundle.loadResourceBundle has the line "table.contains" which causes it to reload the resource bundle for each call.

Changing it to "table.containsKey" fixes the problem.

 

Thanks,

Krishna

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