On Wed, 24 May 2023 15:48:34 GMT, Alexey Ivanov <[email protected]> wrote:
>> I understand that, virtual folders don't have a filesystem path, this is why >> `fsv.isFileSystem(f)` should handle this situation. The implementation of >> `FileSystemView.isFileSystem` explicitly excludes such files which are both >> `isLink` and `isDirectory`: >> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java#L382-L388 >> >> Let's look deeper: >> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L591-L597 >> >> https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L860-L866 >> >> The `hasAttribute` method uses `getAttributes0` which calls >> [`IShellFolder::GetAttributesOf`](https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellfolder-getattributesof) >> which returns >> [`SFGAO`](https://learn.microsoft.com/en-us/windows/win32/shell/sfgao). The >> attribute `SFGAO_LINK` which corresponds to `ATTRIB_LINK` in Java is >> described as _“The specified items are shortcuts.”_ That is it is a shell >> folder shortcut with `.lnk` file extension, which used to be the case. >> >> However, now this attribute is set for both [_symbolic >> links_](https://learn.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links) >> and >> [_junctions_](https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions#junctions). >> >> Here's what different methods return: >> >> file: C:\test\symbolic-links\jdk - junction >> isFileSystem false >> isLink true >> isDirectory true >> isSymbolicLink false >> Absolute Path : null >> >> file: C:\test\symbolic-links\jdk - symbolic >> isFileSystem false >> isLink true >> isDirectory true >> isSymbolicLink true >> Absolute Path : C:\test\symbolic-links\jdk - symbolic >> >> file: C:\test\symbolic-links\jdk - shortcut.lnk >> isFileSystem false >> isLink true >> isDirectory true >> isSymbolicLink false >> Absolute Path : null >> >> >> In the three cases above, `isFileSystem` returns `false` even though all the >> three objects point to a filesystem directory, the first two are fully >> transparent for apps whereas the `.lnk` shortcut needs special handling. >> >> If `JFileChooser` is in directory-only mode, selecting a junction or `.lnk` >> file results in selected file being to `null`; symbolic links are support... > > Handling `.lnk` files on Windows doesn't seem right: `JFileChooser` allows > selecting such a file. > > * If it's a shortcut to a directory, clicking Open in the file chooser or > double-clicking it in the list navigates to the target. > * If it's a shortcut to a regular file, `JFileChooser` returns this file > which isn't helpful, it's not what the user usually wants, the target of the > shortcut should be returned. This is what happens if the user selects a > shortcut file (`.lnk`) in the native file chooser. > > In addition to that, the shortcuts are filtered based on the file types. For > example, if select the *Open* command in Notepad, the file chooser shows only > shortcuts to `.txt` files (as well as `.txt` files), it hides shortcuts to > other file types. > > But `JFileChooser` doesn't show `.lnk` files at all if an extension filter is > set. > I understand that, virtual folders don't have a filesystem path, this is why > `fsv.isFileSystem(f)` should handle this situation. The implementation of > `FileSystemView.isFileSystem` explicitly excludes such files which are both > `isLink` and `isDirectory`: > > https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java#L382-L388 > > Let's look deeper: > > https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L591-L597 > > https://github.com/openjdk/jdk/blob/d7245f70e7bac1236bbcdcd9b25346ca22ab8bb2/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L860-L866 > > The `hasAttribute` method uses `getAttributes0` which calls > [`IShellFolder::GetAttributesOf`](https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishellfolder-getattributesof) > which returns > [`SFGAO`](https://learn.microsoft.com/en-us/windows/win32/shell/sfgao). The > attribute `SFGAO_LINK` which corresponds to `ATTRIB_LINK` in Java is > described as _“The specified items are shortcuts.”_ That is it is a shell > folder shortcut with `.lnk` file extension, which used to be the case. > > However, now this attribute is set for both [_symbolic > links_](https://learn.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links) > and > [_junctions_](https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions#junctions). > > Here's what different methods return: > > ``` > file: C:\test\symbolic-links\jdk - junction > isFileSystem false > isLink true > isDirectory true > isSymbolicLink false > Absolute Path : null > > file: C:\test\symbolic-links\jdk - symbolic > isFileSystem false > isLink true > isDirectory true > isSymbolicLink true > Absolute Path : C:\test\symbolic-links\jdk - symbolic > > file: C:\test\symbolic-links\jdk - shortcut.lnk > isFileSystem false > isLink true > isDirectory true > isSymbolicLink false > Absolute Path : null > ``` > > In the three cases above, `isFileSystem` returns `false` even though all the > three objects point to a filesystem directory, the first two are fully > transparent for apps whereas the `.lnk` shortcut needs special handling. > > If `JFileChooser` is in directory-only mode, selecting a junction or `.lnk` > file results in selected file being to `null`; symbolic links are supported > with your fix. > > So, **I propose modifying the implementation of > `FileSystemView.isFileSystem`** so that it returns only `sf.isFileSystem()` > or filters out the `.lnk` shortcuts only. Since `FileSystemView.isFileSystem` is used in many places for particular Look and Feel, there might be chances of regression. So how about checking for shell folder first and if yes then can combine the `shell.isFileSystem()` with `isSymbolicLink()` in BasicFileChooser class.......? If its not a shell folder then directly we can check for `isSymbolicLink()`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1205092918
