RFR: 8288893: Popup and its subclasses cannot input text from InputMethod [v3]
Martin Fox
mfox at openjdk.org
Wed Dec 11 17:50:27 UTC 2024
On Wed, 4 Dec 2024 14:39:55 GMT, Martin Fox <mfox at 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
More information about the openjfx-dev
mailing list