7186009: [macOS] No way to hide(), dispose() FileDialog#30715
Open
prrace wants to merge 2 commits intoopenjdk:masterfrom
Open
7186009: [macOS] No way to hide(), dispose() FileDialog#30715prrace wants to merge 2 commits intoopenjdk:masterfrom
prrace wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back prr! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are about 26 modal FileDialog tests marked as failing on macOS due to this bug.
They test the various modality cases for FileDialog.
The missing macOS support for FileDialog.dispose() affects these tests since they require dispose()
to work to dismiss the dialogs after checking the modality support.
In the macOS native code the FileDialog (an NSOpenPanel) is displayed by calling a macOS API "runModalWithDirectory". So it is modal.
That runModalWithDirectory API is deprecated, but the replacement "runModal" is the same as far as this issue goes.
There is no macOS API to directly hide or progammatically "cancel" the dialog. In fact the latter is explicitly disallowed.
But I have found that there is a macOS API "stopModal" which will break the loop and dismiss the dialog.
It seems to work well for these tests. There is a corner-case issue that stopModal doesn't take a window parameter.
Therefore care needs to be taken so that we only stop the loop if the current modal window is the one for this FileDialog.
24 of the 26 tests assume application modal behaviour of the dialog and pass with that fix.
Also the ones marked as failing on Linux haven't failed at all for me in all my test runs, so all of these are de-problem listed.
The 2 that still fail because they require docmodal/modeless are marked as !Mac
Per the spec there's no requirement to support this, even though apparently Linux & Mac do.
class doc says : "The FileDialog class displays a dialog window from which the user can select a file.
Since it is a modal dialog, when the application calls its show method to display the dialog, it blocks the rest of the application until the user has chosen a file."
The change to test !inDispose is fully explained in the JBS issue, but briefly it is required to prevent
the thread running the dialog from hanging if dispose is called by the application.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30715/head:pull/30715$ git checkout pull/30715Update a local copy of the PR:
$ git checkout pull/30715$ git pull https://git.openjdk.org/jdk.git pull/30715/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30715View PR using the GUI difftool:
$ git pr show -t 30715Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30715.diff
Using Webrev
Link to Webrev Comment