RFR: 8312075: FileChooser.win32.newFolder is not updated when changing Locale [v4]

Alexey Ivanov aivanov at openjdk.org
Wed Aug 30 14:50:19 UTC 2023


On Thu, 10 Aug 2023 07:39:06 GMT, Tejesh R <tr at openjdk.org> wrote:

>> On `NewFolderAction`, plain String is added `Action.ACTION_COMMAND_KEY`. Converting the `String `to `locale` before adding as command key fix the issue. 
>> I have verified the test in all other platforms and Look and Feel which has option to create New Folder, results were fine. No regressions found on CI system with the fix. Added manual test to verify the fix.
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Test update

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 1:

> 1: /*

Update the copyright year.

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 785:

> 783:         String newFolderString =
> 784:                 UIManager.getString("FileChooser.other.newFolder");
> 785:         String newFolderNextString  =

Suggestion:

        String newFolderNextString =

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 36:

> 34:  * @summary Test to check if created newFolder is updated with
> 35:  *          changing Locale.
> 36:  * @run main FileChooserNewFolderLocaleTest

Suggestion:

 * @run main/othervm -Duser.language=en -Duser.country=US  FileChooserNewFolderLocaleTest

To ensure the test starts with US English locale.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 42:

> 40:     static ResourceBundle res;
> 41:     static String newFolderKey;
> 42:     static String newFolderSubKey;

These could be local variables if you inline the `setLocale` method.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 46:

> 44:     public static void main(String[] args) throws Exception {
> 45:         File newFolderEnglish = null;
> 46:         File newFolderFrench = null;

~~Declare variables where they're used for the first time.~~

I see, you use them for cleaning up in the finally block.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 64:

> 62: 
> 63:             setLocale("fr");
> 64:             fileChooser = new JFileChooser();

You should be able to re-use the same instance.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 68:

> 66:                     fileChooser.getFileSystemView().createNewFolder(currentDir);
> 67:             if( !newFolderFrench.getName().contains
> 68:                     (UIManager.getString(newFolderKey))) {

Suggestion:

            if (!newFolderFrench.getName().endsWith(UIManager.getString(newFolderKey))) {


Better yet, create a constant for `FRENCH_NEW_FOLDER = "Nouveau dossier"` and use it to put the value to `UIManager` as well as to check it's used.

You may want to ensure, `newFolderEnglish` ends with `"New Folder"`.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 78:

> 76:                 System.out.println("Failed to delete file : " +
> 77:                         newFolderEnglish.getName());
> 78:             }

Perhaps, you don't need to print anything here at all or report an error only.

test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java line 114:

> 112:                 res.getString("new_folder"));
> 113:         UIManager.put(newFolderSubKey,
> 114:                 res.getString("new_folder") + " ({0})");

The only thing needed here is:

        UIManager.put(newFolderKey, "Nouveau dossier");
        UIManager.put(newFolderSubKey, "Nouveau dossier ({0})");

And this could be inlined into the test method code.

-------------

PR Review: https://git.openjdk.org/jdk/pull/15069#pullrequestreview-1602811647
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310386196
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310348256
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310379187
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310395036
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310360687
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310361661
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310377988
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310397046
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310371568


More information about the client-libs-dev mailing list