On Wed, 4 Dec 2024 14:39:55 GMT, Martin Fox <m...@openjdk.org> wrote:
>> 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.
> Why do we need to keep a list of scenes? Can't we get this information
> indirectly `from Window.getWindows()` ?
We need to recalculate the IM state whenever a PopupWindow is shown or hidden
(because the set of focusOwners changes). In this PR I'm relying on the
existing logic in the PopupWindow class to track when scenes are added or
removed so I can match when the PopupWindow starts and stops event monitoring.
This is done via the addScene and removeScene calls in the
InputMethodStateManager.
In theory I could track the coming and going of scenes by monitoring window
ownership lists. This would be complicated since it's recursive. The root
scene's window only owns the first popup. If another popup is shown it's owned
by the popup below it. I would have to attach change listeners to the root
scene's window list, scan the list for popups, and then recursively add change
listeners to their window lists. I would much rather track what the PopupWindow
class is already doing rather than try to implement an entirely separate
mechanism.
Given that I rely on the PopupWindow code to tell me when scenes come and go
it's expedient to just keep a list of the scenes around. I suppose I could
remove that list and instead work my way recursively through the window
ownership lists but I don't think there's much to be gained by that.
> 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?
The listeners attached to the focusOwner property are triggered too late in the
process. The current system updates the OS state (via
finishInputMethodComposition and enableInputMethodEvents) at very specific
points in time and I don't want to disturb that. In particular all of this
happens before we trigger the change listeners on the focusOwner property.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1634#discussion_r1880650779
