Added an option to overrule the last file dialog location on Windows

[Mailaender]

As suggested in https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190 I made it configurable, so hopefully everyone is happy. The main reason is that it is inconsistent across platforms (only Windows "misbehaves") and your application may want to store the last saved location on its own per perspective / data type etc.

To test, run Snippet72 on Windows. Select a file from another folder say C:\Windows\win.ini. Run it twice to verify it uses the preset location. Remove setFilterForcePath to bring back the legacy behavior where it by default will just ignore the set filter path if you used any save dialog before.

[github-actions]

Test Results

     303 files       303 suites   7m 19s ⏱️
  4 047 tests   4 035 ✔️   8 💤 3 ❌ 1 🔥
12 068 runs  11 993 ✔️ 70 💤 4 ❌ 1 🔥

For more details on these failures and errors, see this check.

Results for commit f0dc0a8.

♻️ This comment has been updated with latest results.

[lshanmug]

Thanks for the PR to fix the issue.

Do we need a new API here to fix the Windows behavior?
AFAICS from the comments in https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c7, setFilterPath() worked as a hard override until 4.20. I couldn't verify it myself as I don't have Windows, may you can do a quick check? If that's the case, we should directly fix setFilterPath() instead of adding a new API. Also, that will make the setFilterPath() API consistent on all platforms.

[Mailaender]

When I change the default way on Windows, someone will only revert it back as in #142. That is why I added an optional switch. However, I am open to suggestions on how to change the API.

[lshanmug]

When I change the default way on Windows, someone will only revert it back as in #142. That is why I added an optional switch. However, I am open to suggestions on how to change the API.

AFAICS from https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c17 the change was reverted because it caused 2 other issues as mentioned in the comment there.
We should be good to fix the default behavior of setFilterPath() on Windows, after verifying that:

• setFilterPath() worked as a hard override until 4.20 (by testing any one version < 4.20)
• The 2 issues mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c17 don't reappear after the fix.

[Mailaender]

Is there consensus for setFilterForcePath defaulting to true?

[lshanmug]

AFAICS from https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c17 the change was reverted because it caused 2 other issues as mentioned in the comment there. We should be good to fix the default behavior of setFilterPath() on Windows, after verifying that:

* setFilterPath() worked as a hard override until 4.20 (by testing any one version < 4.20)

* The 2 issues mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c17 don't reappear after the fix.

Can you pls verify your fix based on above points?

Is there consensus for setFilterForcePath defaulting to true?

Please see #466 (comment) above, the fix should go into setFilterPath instead of new API.

[plexiq]

If you make it configurable then I think there's a strong case for defaulting setFilterForcePath to true, that way the behavior is consistent across platforms unless specifically changed.

It would be preferable to simply revert the behavior to <4.20 (ie setFilterForcePath = true) without an API change. The sole argument I can see for making this configurable would be to reduce the possibility of this change being reverted again, the option doesn't appear to be useful beyond that.

[dhendriks]

Let me try to summarize all information, as I'm getting lost in all the spread-out information in various issue, pull requests, etc.

Background:

• SWT migrated FileDialog to COM-based API, which may or may not be related (https://bugs.eclipse.org/bugs/show_bug.cgi?id=571571)

There may be a related or unrelated issue with root / 'This PC' / 'Computer' directory:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=579359#c0
• FileDialog does not remember previous directory #95 (comment)

Potential reasons to make it always a hard override:

• A soft override is unexpected:
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190 asks for this (I created this issue).
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=578045 (closed duplicate of 577190, created by a different person)
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=578045#c2 (confirmed by yet a different person as being unexpected)
• To make it consistent accross platforms:
  • Added an option to overrule the last file dialog location on Windows #466 (comment) ("it is inconsistent across platforms")
  • Added an option to overrule the last file dialog location on Windows #466 (comment) ("will make the setFilterPath() API consistent on all platforms")
• Supports requested use cases:
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=579359 expects for repeated import to start at "folder referenced by the currently selected user library", which could be implemented using a hard override to that folder.
  • Added an option to overrule the last file dialog location on Windows #466 (comment) ("your application may want to store the last saved location on its own per perspective / data type etc")

Potential reasons for reverting the change and keeping the current behavior, or supporting a hybrid approach as per this pull request:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c17
  • Indicates "it doesn't match the historical (non-COM) FileDialog behavior". This may or may not be true, see below.
  • Refers to other issues. I listed their relevant information separately.
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=579359
  • Wants to repeatedly import from the same place.
  • Not sure whether Windows dialog remember-behavior is the only way to achieve this. Maybe they can just set a specific folder that they want? (see also above)
• FileDialog does not remember previous directory #95
  • They want that the previous directory is remembered. No practical use case is mentioned.
  • Not sure to what path the filter path is set? FileDialog.setFilterPath(null); is considered a workaround, so some path must be set (see FileDialog does not remember previous directory #95 (comment)). I wonder if they could set the path to what they want explicitly?

Regarding whether this is a regression, there is mixed evidence:

• Evidence for it being a regression:
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c7 (hard override on SWT 4.9, soft override on 'latest on 2021-12-16')
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19 (hard override 4.16, 4.17, 4.18, 4.19, 4.23 / soft default 4.20, 4.21, 4.22, 4.24, 4.25M1)
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=578045#c0 (after an upgrade to Eclipse 2021-09 it works differently)
• Evidence for it not being a regression:
  • https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c2 (soft override on both Eclipse Platform 2020-06/4.16 and Eclipse Platform 2021-06/4.20)
  • [Win32] Restore historical FileDialog behavior #142 (comment) (hard override is reverted: "restores original behavior")

Options for the design:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c6 (single method with two overloads, essentially an optional parameter)
• Added an option to overrule the last file dialog location on Windows #466 (two separate methods)

Possible implementations for hard override:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=578045#c0 (call ClearClientData)
• Added an option to overrule the last file dialog location on Windows #466 (SetFolder vs SetDefaultFolder)

Possible defaults for a hard override option:

• https://github.com/eclipse-platform/eclipse.platform.swt/pull/466/files#diff-445b4a948a0de5818c338e63e2a5f3d6f8d03d29bb869480ba75d8ec995fbc41R47 (no hard override by default, current state of PR)
• Added an option to overrule the last file dialog location on Windows #466 (comment) (hard override by default, requested)

[plexiq]

There appears to be conflicting evidence regarding the behavior on 4.16:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c2 (@dhendriks found 4.16 to work as a soft override)
https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19 (In the tests I previously posted, 4.16 was working as a hard override)

I have re-run the tests for 4.16 and additional ones for 4.13, 4.14 and 4.15. All of these versions work as a hard override for me on Windows 10, using the same snippet for testing (https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19).

Can someone please help verify if we actually have inconsistent behavior on 4.16, or if the previous report of 4.16 working as a soft default was a mistake?

[dhendriks]

There appears to be conflicting evidence regarding the behavior on 4.16: https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c2 (@dhendriks found 4.16 to work as a soft override) https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19 (In the tests I previously posted, 4.16 was working as a hard override)

https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c19 uses two different paths for the setFilterPath calls. According to https://bugs.eclipse.org/bugs/show_bug.cgi?id=577190#c3, Windows remembers the path only if the same path is given. What happens if you change the snippet to provide C:\\ both times as filter path, and still the first time select C:\\Windows\\notepad.exe?

[plexiq]

What happens if you change the snippet to provide C:\ both times as filter path, and still the first time select C:\Windows\notepad.exe?

I can not reproduce this behavior. Calling with C:\\ twice, selecting C:\\Windows\\notepad.exe the first time, the second dialog still opens as C:\\ (ie, hard override) using 4.16. The snippet is unchanged, except for the change to C:\\.

Tested with: https://archive.eclipse.org/eclipse/downloads/drops4/R-4.16-202006040540/

Update: I'm actually getting inconsistent results and can sometimes reproduce the behavior you describe when switching around between the JDK versions used to compile and/or run. But using SWT 4.16 and compiling and running with Java-17 results in a hard override, even when calling twice with C:\\.

[Mailaender]

I have a hard time seeing how to proceed from all those conflicting comments. I now merged the parameter into the existing setFilterPath hoping this is what you want when asking for no API change. I can also change the defaults on Windows, but I am not sure what is preferred.

[plexiq]

I think the main priority is to get back some option to achieve "override" behavior on all systems, whether that's by default or by using an additional parameter isn't overly important imo.

From my point of view it would be preferable to have setFilterForcePath = true as default on Windows, as this would result in consistent behavior across OSX/Linux/Windows, but this isn't a big issue either way as long as an option exists.

[Mailaender]

Set filterForcePath = true for consistent behavior across platforms as suggested.

[lshanmug]

AFAIU the main usecase for having an API is to provide an option in FileDialog to store the last saved location. But, that should be the default behavior of FileDialog, it should remember the last opened directory see issue

So, 2 changes are required in FileDialog on Windows here:

1. Fix setFilterPath() to set the dialog's directory path to as a hard override.
2. Keep the default behavior of FileDialog to remember the last opened directory. This eliminates the need for an API to control this. setFilterPath(null) would have the same behavior.

This would fix the current problem with FileDialog.setFilterPath(), make it consistent across all platforms and provides a way for users to switch between the behaviors.

[Mailaender]

Note: there is also a competitive #530.

[Mailaender]

I am not sure if I understand your suggested changes. Added a new change set which hopefully goes into the right direction.

[lshanmug]

I am not sure if I understand your suggested changes. Added a new change set which hopefully goes into the right direction.

Thanks, the change is in the right direction, but point 2 below is not covered. Let me explain.

2. Keep the default behavior of FileDialog to remember the last opened directory. This eliminates the need for an API to control this. setFilterPath(null) would have the same behavior.

With the current PR, if a FileDialog is opened without any call to setFilterPath(), the FileDialog would open at the Windows "root" directory and last opened directory will not be remembered, hence bringing back #95. For example: try the snippet in #95 (comment).
From the code, the default filter path being used in FileDialog is "" and not null (line 52). So, the else part in your PR (line 301) would not be called in the default case (when setFilterPath is not called).

@deepika-u, I don't have Windows to test the PR, can you pls give it a try?

[tmssngr]

IMHO the path remembering only should happening if setFilterPath not is invoked. Honestly, I don't really understand the need for this request at all, because remembering the path easily could be handled by client code in a much more flexible way, e.g. for different kind of files different paths could be remembered.

[lshanmug]

IMHO the path remembering only should happening if setFilterPath not is invoked. Honestly, I don't really understand the need for this request at all, because remembering the path easily could be handled by client code in a much more flexible way, e.g. for different kind of files different paths could be remembered.

@tmssngr Not sure which request you are referring to. My comment was requesting to remember the path when setFilterPath is not invoked, otherwise it would cause #95.

[deepika-u]

@Mailaender

I have tested your new change that fixes the original issue but when filterPath is not set, we are loosing the original behavior as conveyed by @lshanmug(it is not remembering the history of last saved path).

[lshanmug]

@Mailaender

I have tested your new change that fixes the original issue but when filterPath is not set, we are loosing the original behavior as conveyed by @lshanmug(it is not remembering the history of last saved path).

@deepika-u Thanks for testing.

Will this fix the problem?

• In default case, when setFilterPath() is not called, filterPath == "" -> call SetDefaultFolder
• When filterPath != null -> call SetFolder

[deepika-u]

@Mailaender
I think i have a fix covering these 2 test cases, will test it once more and share the fix details.

[deepika-u]

The needed changes are tested and merged with pr => #556

Thanks everyone! please let me know if that works for you.

[lshanmug]

The needed changes are tested and merged with pr => #556

Thanks everyone! please let me know if that works for you.

Thanks @deepika-u for the fix!

Can you please test with Snippet 72 with different combinations of setFilterPath() ?

setFilterPath(path)
setFilterPath(null)

Also, pls verify in Eclipse with tomorrow's I-build.

[deepika-u]

Test cases i tried are as below ::
Via Snippet72 :

1. Set a valid filterPath(say for example "c:\")
2. Set an invalid filterPath(say for example "c:\@123")
3. Set filterPath to space(filterPath = " ")
4. Set filterPath to null(filterPath = null)
5. filterPath not set.
6. Set valid path then immediately set to null, then test.
7. Set to null then immediately set to valid path, then test.

General testing :
8. Windows > Preferences > User Libraries > Import > Browse > select some jar file(by changing .), open. Now Browse again and you'll notice path is remembered now.

This issue is fixed via the pr #556 => #556

The fix is now available in RC1.

You may please test this fix and let us know your feedback.

[akurtakov]

Alternative fix #556 is in.