On Tue, 3 Dec 2024 22:06:48 GMT, Andy Goryachev <[email protected]> wrote:
>> Martin Fox has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Tweak to satisfy system test.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/InputMethodStateManager.java
> line 48:
>
>> 46: * PopupWindow.
>> 47: */
>> 48: public class InputMethodStateManager {
>
> All this feels overly complicated -
>
> Why do we need to keep a list of scenes? Can't we get this information
> indirectly `from Window.getWindows()` ?
>
> Since the `PopupWindow` has the `ownerWindowProperty`, could we simply listen
> to the focusOwner property and get the "root" window and its scene by
> traversing the hierarchy? Do we really need to maintain derived lists when
> the InputMethodRequests is only needed during the handling of a specific
> event?
I don't like trying to maintain derived lists either. What you're proposing is
an interesting idea but I won't have time to work out the details until next
week.
> modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java line 578:
>
>> 576: // still monitoring owner events.
>> 577: Scene scene = getScene();
>> 578:
>> SceneHelper.getInputMethodStateManager(scene).removeScene(scene);
>
> `Window.scene` is a property, so theoretically the application code can set a
> new Scene at any moment. How will that be handled?
In a PopupWindow the scene there is no API to set the scene. It's managed
internally by the system and should not change over the lifetime of the popup
window. I will take a second look at that code next week to verify.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1634#discussion_r1869663328
PR Review Comment: https://git.openjdk.org/jfx/pull/1634#discussion_r1869663211
