RFR: 8288893: Popup and its subclasses cannot input text from InputMethod [v3]

[Andy Goryachev]

On Tue, 12 Nov 2024 15:08:11 GMT, Martin Fox <mfox at openjdk.org> wrote:

>> Input methods don’t work for text controls inside Popups. This PR fixes that.
>> 
>> Some background:
>> 
>> A PopupWindow always has an owner. The owner of the first Popup is a standard Window; I’ll refer to that as the root window. But a popup can show another popup (for example, a popup may contain a ComboBox which opens a menu which is also a Popup) resulting in a stack of PopupWindows.
>> 
>> Under the hood each PopupWindow has its own scene (this is not visible in the API). So if there’s a stack of PopupWindows there’s also a stack of scenes with the root window’s scene at the bottom.
>> 
>> The OS focus always stays with the root window (in part because JavaFX can’t move the OS focus when it’s embedded). This means a KeyEvent is initially fired at the focusOwner in the root window’s scene. Each PopupWindow in the stack uses an EventRedirector to refire that key event at its own focusOwner. In effect KeyEvents start processing at the top-most scene in the stack and work their way down to the root scene.
>> 
>> There are several reasons why Input Methods aren’t currently working for Popups.
>> 
>> - InputMethodEvents are not being redirected. This PR extends the EventRedirector to refire InputMethod events in the same way it refires KeyEvents.
>> 
>> - If a PopupWindow contains a TextInput control it will enable input method events on its scene which has no effect since that scene doesn’t have OS focus. If a PopupWindow wants to enable IM events it needs to do so on the root window’s scene. Put another way IM events should be enabled on the root scene if and only if one of the focusOwners in the scene stack requires IM events.
>> 
>> - The OS always retrieves the InputMethodRequests from the root window’s scene. InputMethodRequests should be retrieved from whichever focusOwner in the scene stack is processing InputMethodEvents.
>> 
>> In this PR the root scene creates an InputMethodStateManager object and shares it with all of the Popup scenes in the stack. Any time the focusOwner changes in a scene it tells the InputMethodStateManager so it can determine whether IM events should be enabled on the root scene. The root scene also calls on the InputMethodStateManager to retrieve InputMethodRequests so it can grab them from the appropriate Node in the scene stack.
>> 
>> This PR also fixes JDK-8334586. Currently the scene only enabled or disables IM events when the focusOwner changes. If a node’s skin is installed after it becomes focusOwner the scene won’t notice the cha...
>
> 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?

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?

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1634#discussion_r1868427884
PR Review Comment: https://git.openjdk.org/jfx/pull/1634#discussion_r1868400092