On Mon, 10 Oct 2022 13:58:30 GMT, Tejesh R <t...@openjdk.org> wrote: >> When a custom `FileView` is used and folder traversal is restricted to a >> particular directory NPE occurs when user tries to traverse/select other >> folders except traversable folder. This is caused because when user selects >> folder other than traversable, the traversal is rejected and hence no file >> is selected as `currentDirectory` of `JFileChooser`. When user tries to >> access the restricted folder second time, previous selected file check is >> failing because of NPE since `getFileChooser().getCurrentDirectory();` is >> null. To fix the issue, NPE check is added. > > Tejesh R has updated the pull request incrementally with one additional > commit since the last revision: > > Updated based on review comments
Changes requested by aivanov (Reviewer). test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 35: > 33: > 34: import javax.swing.filechooser.FileView; > 35: import jdk.test.lib.Platform; It's not used any more, please remove. Leave one blank line after the last import and the following comment with the jtreg tags. test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 42: > 40: * @requires (os.family == "windows" | os.family == "linux") > 41: * @library /java/awt/regtesthelpers /test/lib > 42: * @build PassFailJFrame jdk.test.lib.Platform Suggestion: * @library /java/awt/regtesthelpers * @build PassFailJFrame `Platform` class from `/test/lib` is not used any more. test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 65: > 63: JFileChooser jfc; > 64: > 65: //Initialize the components I believe the comment is redundant. Being placed before the instruction text, it's confusing. The method name `initialize` is self-explanatory. test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 77: > 75: 3. If NPE occurs on second selection then test FAILS, > else test > 76: is PASS. > 77: """; I propose the following text: Instructions to Test: 1. The traversable folder is set to the Documents folder, if it exists, in the user's home folder, otherwise it's the user's home. Other folders are non-traversable. 2. When the file chooser appears on the screen, select any non-traversable folder from "Look-In" combo box, for example the user's folder or a folder above it. (The folder will not be opened since it's non-traversable). 3. Select the Documents folder again. 4. If NullPointerException occurs in the step 3, click Fail, otherwise click Pass. It consistently uses _“folder”_, no mention of “directory”. It explains that the user's home could be the root of the traversable tree. The instructions use imperative to select either _Fail_ or _Pass_. test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 81: > 79: passFailJFrame = new PassFailJFrame("Test Instructions", > INSTRUCTIONS, 5L, 13, 40); > 80: jfc = new JFileChooser(); > 81: String path = System.getProperty("user.home") + File.separator + > "Documents"; Suggestion: String userHome = System.getProperty("user.home"); String docs = userHome + File.separator + "Documents"; String path = (new File(docs).exists()) ? docs : userHome; This will make the test more robust, it can continue to run even if `Documents` folder does not exist. test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 84: > 82: > 83: jfc.setCurrentDirectory(new File(path)); > 84: jfc.setFileView(new CustomFileView(path)); Suggestion: jfc.setFileView(new CustomFileView(path)); jfc.setControlButtonsAreShown(false); Hide _Open_ and _Cancel_ buttons in the file chooser. ------------- PR: https://git.openjdk.org/jdk/pull/10485