RFR: 8281966: Absolute path of symlink is null in JFileChooser [v2]

[Alexey Ivanov]

On Mon, 1 Aug 2022 10:27:03 GMT, Tejesh R <tr at openjdk.org> wrote:

>> Absolute path of Symbolic Link created in Windows is set to `null` in `BasicFileChooserUI` class. This happens when propertyChangeListener is implemented to get the Symbolic link's Absolute path on Mouse click through JFileChooser. The reason being that on click of Symbolic link, the _ValueChanged()_ in `BasicFileChooserUI` class has a logic which actually sets the  `chooser.SelectedFile()` to `null` even though the path is not null. Hence the issue is addressed by checking if its a Symbolic link and then setting the `chooser.SelectedFile()` to the value of clicked link without modifying the other logics.
>
> Tejesh R has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix : Updated to handle multiSelection case
>  - Manual Test Indicator Added in JTREG tag

It does not work.

In the single-selection case, I see the following output:


Absolute Path : null
Absolute Path : C:\filechooser\link


Above, `null` corresponds to `C:\filechooser\target` folder which is the target of the `link`.

Neither does it work in the multi-selection case:


Absolute Path : C:\filechooser
Absolute Path : null
Absolute Path : C:\filechooser\target


But now it doesn't work in a different way: `link` is reported as `null`.


I can't see none of the comments addressed in the test even though you explicitly added comments that you updated the code. A missing push?

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

> 57:                 } catch (InvocationTargetException e) {
> 58:                     throw new RuntimeException(e);
> 59:                 }

The handlers are the same, collapse the catch block into one which handles both exceptions.

test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java line 64:

> 62: 
> 63:         robot.delay(2000);
> 64:         robot.waitForIdle();

Robot is not required. You can safely remove these two lines.

The following line just starts to wait for the user to press `Pass` or `Fail`.

test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java line 86:

> 84:         PassFailJFrame.positionTestWindow(frame, PassFailJFrame.Position.HORIZONTAL);
> 85: 
> 86:         frame.setPreferredSize(new Dimension(600, 600));

You use `pack` below, so it seems redundant.

test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java line 95:

> 93:             public void propertyChange(PropertyChangeEvent evt) {
> 94:                 if (JFileChooser.SELECTED_FILE_CHANGED_PROPERTY.equals(evt.getPropertyName())) {
> 95:                     System.out.println(String.format("Absolute Path : %s",evt.getNewValue()));

When with jtreg, the output to the console is not seen.

You have to provide a UI where these lines would be added.

The usage of `format` for such a simple case could be replaced by string concatenation using `+`.

In addition to that, you may keep track whether a `null` value is seen or not. No `null` value is expected. If it's seen, it's a failure.

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

Changes requested by aivanov (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9597