On Mon, 19 May 2025 13:52:16 GMT, Tejesh R <t...@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
