Re: [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the ResourceBundle for every call to toDisplayString

[Philip Race]

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: 
http://cr.openjdk.java.net/~kaddepalli/8197785/webrev01/ 
<http://cr.openjdk.java.net/%7Ekaddepalli/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 <krishna.addepa...@oracle.com>
*Cc:* swing-dev@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: http://cr.openjdk.java.net/~kaddepalli/8197785/webrev00/
    <http://cr.openjdk.java.net/%7Ekaddepalli/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