-
Notifications
You must be signed in to change notification settings - Fork 476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8331319: IME Window breaks after popup menu #1447
Conversation
👋 Welcome back mfox! A progress list of the required criteria for merging this PR into |
@beldenfox This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
Reviewers: @andy-goryachev-oracle @kevinrushforth /reviewers 2 |
@kevinrushforth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I verified on my macOS 13.6.6 system that this fixes the problem. I also verified that the test program from JDK-8320912 still works correctly.
Seems to work with the little test program. Observed a weird behavior in the Monkey Tester
It seems to me it might be using font from the previous invocation? |
That seems likely a different issue. Can you try it with a build without the current PR? And maybe also try it with JavaFX 22 to rule out other recently-introduced problems? |
same issue without the fix. I agree - this is likely unrelated. I won't be able to try with an earlier jdk in the next couple of hours, sorry. |
With jfx21-ea+12, I see the same incorrect font, so it must be a different issue.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ship it!
/integrate |
Going to push as commit cac81b5. |
@beldenfox Pushed as commit cac81b5. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
When focus moves away from a node JavaFX calls
finishInputMethodComposition
so glass can clean up any in-progress IME editing. On Mac we calldiscardMarkedText
on the view's NSTextInputContext to dismiss the IME.It appears that the OS can get confused if
discardMarkedText
is called on an NSTextInputContext that is not the current active context. JavaFX popups are displayed in windows that don't have OS focus and therefore do not have the active input context but the JavaFX scene associated with the popup doesn't know this and still makes calls to manipulate the IME. This seems to be triggering a bug in the OS that leads to bad behavior which persists until the user moves focus away from the main JavaFX stage altogether and then brings it back.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1447/head:pull/1447
$ git checkout pull/1447
Update a local copy of the PR:
$ git checkout pull/1447
$ git pull https://git.openjdk.org/jfx.git pull/1447/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1447
View PR using the GUI difftool:
$ git pr show -t 1447
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1447.diff
Webrev
Link to Webrev Comment