RFR: 8305072: Win32ShellFolder2.compareTo is inconsistent [v2]

[Alexey Ivanov]

On Tue, 12 Mar 2024 19:35:30 GMT, Phil Race <prr at openjdk.org> wrote:

>> Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Handle fakePersonal on Windows 10
>>   
>>   The desktop folder on Windows 10 does not include
>>   the Personal (Documents) folder.
>
> src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java line 525:
> 
>> 523:         boolean special2 = sf2.isSpecial();
>> 524: 
>> 525:         if (special1 && special2) {
> 
> Could you please say something on how/why this change solves it ?

It is well described in a [JBS comment](https://bugs.openjdk.org/browse/JDK-8305072?focusedId=14610400&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14610400) by the submitter.

Let's consider the failing case where:

a = C:\Users<user>\Documents(true)
b = C:\Users<user>(false)
c = C:\Users<user>\Documents(false)

The value in parentheses is `isSpecial()` flag.

When `a` and `c` are compared, `special1 = true` and `special2 = false`, therefore the code flow enters this `if`-block. Because the `equals` method doesn't take into account the `isSpecial` flag, both `i1` and `i2` are 0.

https://github.com/openjdk/jdk/blob/7bccc847fe9e8c5e640d8fa9c93ce32563a0ccfa/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java#L534-L535

Thus, `a` and `c` are considered equal, which breaks the general contract of [`Comparable.compareTo`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Comparable.html#compareTo(T)).

If you look below this `if`-statement:

https://github.com/openjdk/jdk/blob/7bccc847fe9e8c5e640d8fa9c93ce32563a0ccfa/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java#L545-L550

The comment and the code say that any special folder sorts above any regular folder. Therefore the code shouldn't enter that `if`-block above if only one of the folders is *special* — the order is already well-defined for such case.

The purpose of the `if`-block is to make some of the special folders even more special. In other words, it ensures the following special folders—`getPersonal`, `getDesktop`, `getDrives`, `getNetwork`—are at the top of the list, above any other special folders.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18126#discussion_r1525204254