RFR: 8139228: JFileChooser renders file names as HTML document [v3]

Tue May 20 18:52:55 UTC 2025

On Mon, 19 May 2025 13:52:16 GMT, Tejesh R <tr at openjdk.org> wrote:

>> The rendering of the directory names are handled as JLabel w.r.t Look and feel and also either Details/List view. Though FilePane creates basic rendering for these two few Look and Feel define their own renderers and also ComboBox Directory directory name view. Since HTML filtering is not taken care in any of these renderers, JLabel renders them as HTML document if nothing is set or specified.
>> The fix is to get "html.disable" property from JFileChooser and set the same to JLabel component which renders and set Directory name. Hence applications can either enable/disable this property and control HTML rendering of directory name.
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review fix

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 29:

> 27: import javax.swing.filechooser.FileSystemView;
> 28: import java.io.File;
> 29: import java.util.List;

Suggestion:

import java.io.File;
import java.util.List;
import javax.swing.Icon;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.filechooser.FileSystemView;

We haven't reached an agreement, yet `java.*` packages usually go before `javax.*` packages in the list of imports.

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 45:

> 43:         String INSTRUCTIONS = """
> 44:                 1. FileChooser shows up a virtual directory and file with name
> 45:                    "<html><h1 color=#ff00ff><font face="Comic Sans MS">Testing Name".

The file name is now `Swing Rocks!`

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 51:

> 49:                          ComboBox remains in plain text, then test PASS.
> 50:                          If it appears to be in HTML format with Pink color,
> 51:                          then test FAILS.

Suggestion:

                         ComboBox remains in plain text, then test passes.
                         If it appears to be in HTML format with Pink color,
                         then test fails.

You could be more specific: _“click **Fail**”_ if a condition isn't met. (Otherwise, continue with the test.) End the instructions with _“click **Pass**”_.

Anyway, the instructions are pretty clear as they are, so real change is required.

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 59:

> 57:                          ComboBox remains in HTML format string, then test PASS.
> 58:                          If it appears to be in plain text, then test FAILS.
> 59:                          (Verify for all Look and Feel).

How do I verify for all Look and Feels?

The test has to iterate over all L&F, and repeat the same thing for each L&F.

Ideally, it should be an automated test. However, I'm fine with postponing any efforts to automate the test but I'd like to try to automate it.

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 60:

> 58:                          If it appears to be in plain text, then test FAILS.
> 59:                          (Verify for all Look and Feel).
> 60:                  """;

Suggestion:

                """;

Avoid trailing space, it's not needed.

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 85:

> 83:         frame.pack();
> 84:         return frame;
> 85:     }

Suggestion:

    private static JFrame createFileChooser(boolean htmlEnabled) {
        JFileChooser jfc = new JFileChooser(new VirtualFileSystemView());
        jfc.putClientProperty("html.disable", htmlEnabled);
        jfc.setControlButtonsAreShown(false);

        JFrame frame = new JFrame((htmlEnabled) ? "HTML enabled" : "HTML disabled");
        frame.add(jfc);
        frame.pack();
        return frame;
    }

I think this way is cleaner: create the file chooser, configure it¹; then create the frame, put the file chooser into the frame.

`html_enabled` → `htmlEnabled`, which aligns to the Java style.

¹ I added `jfc.setControlButtonsAreShown(false)` to hide the *Open* and *Cancel* buttons.

test/jdk/javax/swing/JFileChooser/HTMLFileName.java line 96:

> 94:         public File[] getRoots() {
> 95:             return new File[]{
> 96:                     new File("/", "<html><h1 color=#ff00ff><font face=\"Comic Sans MS\">SWING ROCKS!!!111"),

Suggestion:

                    new File("/", "<html><h1 color=#ff00ff><font face="Comic Sans MS">Swing Rocks!"),

Use title case?

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

PR Review: https://git.openjdk.org/jdk/pull/24439#pullrequestreview-2855169377
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098639699
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098604737
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098653423
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098604212
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098598944
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098610838
PR Review Comment: https://git.openjdk.org/jdk/pull/24439#discussion_r2098642534