Desktop. Fix input methods on JBR, disable input methods when we lose focus #881

igordmn · 2023-10-19T11:34:25Z

Rerequest focus on main component when we need to type using input methods

Fixes JetBrains/compose-multiplatform#2628

The issue was because of 2 things:

we used a hack to force a focused event (component.inputContext.dispatchEvent(focusGainedEvent)
JBR added optimization to ignore focus on the same element (thanks @AiHMin for investigation here). In Compose we have only one element.
Because optimization is correct, and the hack depends on the internals, it isn't right to fix it in JBR, we should fix it in Compose. Furthermore, even without JBR changes, this hack didn't complete work - we can't for example use it for disabling input methods (see the next point).

In this PR we also use a hack unfortenutely - we refocus the root component, focusing on invisible component first. That leads to another issue with acccessibility, but we fix it here).

A proper fix should be switching to native code, or making an API in JBR (but other vendors still be unsupported).

Don't show input methods popup if there is no focused TextField

Previously we showed a popup, even if there are no focused textfield:

Now we don't show it if we isn't in a textfield.
P.S. only on Windows/Linux for now, on macOs Swing seems has a bug

Testing

Tested manually on:

Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility enabled/disabled, ComposeWindow/ComposePanel
macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled
Linux, Chinese layout, OpenJDK 17

1. Panel unavailable on Windows at start 2. When focus on Switch or Box(Modifier.clickable), there was "Panel0". Now it say nothing 3. Fix pronouncing the window title on losing focus in TextField after merging #881 (we refocus main window there) ## Testing Manually with accessibility enabled on run1 (Windows, macOs)

1. Windows pronounced "Panel unavailable" at start 2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing. 3. After merging #881 refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown ## Testing Manually with accessibility enabled on run1 (Windows, macOs)

1. Fixes JetBrains/compose-multiplatform#2628 2. Doesn't show input methods popup if there is no focused TextField (only on Windows for now, on macOs, Swing seems has a bug: JetBrains/compose-multiplatform#3839) ## Testing Tested manually on Windows/macOs/Linux, OpenJDK/JBR, Accessibility enabled/disabled

…ixes issues on Windows) This fixes issues on Windows: 1. Windows pronounced "Panel unavailable" at start 2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing. 3. After merging #881 refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown on macOs it doesn't change anything ## Testing Manually with accessibility enabled on run1 (Windows, macOs)

MatkovIvan · 2023-10-20T08:54:39Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeBridge.desktop.kt

    private val platformComponent: PlatformComponent = object : PlatformComponent {
        override fun enableInput(inputMethodRequests: InputMethodRequests) {
            currentInputMethodRequests = inputMethodRequests
            component.enableInputMethods(true)
-            val focusGainedEvent = FocusEvent(focusComponentDelegate, FocusEvent.FOCUS_GAINED)
-            component.inputContext.dispatchEvent(focusGainedEvent)
+            // Without resetting the focus, Swing won't update the status (doesn't show/hide popup)


Is it possible to cover such a case with a test?

There 2 useful tests that could be written:

Checking that some Swing component enables input methods when we focus on a textfield. Unfortunately, Swing doesn't provide any API for that.

Checking Chinese input in integration with Chinise input enabled in the system. But for that we need a Docker container, a task on CI, and a special test annotation @environmenttest.

Other kind of tests aren't useful:

Checking that TextField enables Input methods on ComposeScene level. It isn't very needed right now, as a similar test already exists in Jetpack Compose (that checks that we call appropriate PlatformTextInputService methods).

Injecting fake JComponent, on which we check that we call enableInputMethods. But this bounds the test to the implementation details.

Walingar

Since this is kinda a hack, we need to discuss better solution with JBR team
I am pretty sure that it can shoot somewhere..
But I am ok to have it for now

Walingar · 2023-10-20T13:13:38Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeBridge.desktop.kt

    abstract val component: JComponent
+    val invisibleComponent: Component get() = _invisibleComponent


The class ComposeBridge is internal, so no need to add it for field

Walingar · 2023-10-20T13:16:57Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

@@ -187,6 +188,7 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
        if (bridge == null) {
            bridge = createComposeBridge()
            initContent()
+            super.add(bridge!!.invisibleComponent, Integer.valueOf(1))


Shouldn't we add it to another layer then?

Will it make a difference? super.add(, 0) looks the same to me as when we just add it before the other component.

igordmn · 2023-10-20T14:23:25Z

Since this is kinda a hack, we need to discuss better solution with JBR team

Created https://youtrack.jetbrains.com/issue/COMPOSE-520/Desktop.-Rewrite-input-methods

… issues on Windows) (#885) This fixes issues on Windows: 1. Windows pronounced "Panel unavailable" at start 2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing. 3. After merging #881, we refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown on macOs it changes one thing: 1. When we focus on Switch or Box(Modifier.clickable), now it prounonces that we in scroll area (if we in scroll area). It also pronounces it for other compnents like Button/Text ## Testing Manually with accessibility enabled in run1 (Windows, macOs)

@AiHMin

… focus (#881) ### Rerequest focus on main component when we need to type using input methods Fixes JetBrains/compose-multiplatform#2628 The issue was because of 2 things: - we used a hack to force a focused event (`component.inputContext.dispatchEvent(focusGainedEvent)` - JBR added optimization to ignore focus on the same element (thanks @AiHMin for investigation [here](JetBrains/compose-multiplatform#2628 (comment))). In Compose we have only one element. Because optimization is correct, and the hack depends on the internals, it isn't right to fix it in JBR, we should fix it in Compose. Furthermore, even without JBR changes, this hack didn't complete work - we can't for example use it for disabling input methods (see the next point). In this PR we also use a hack unfortenutely - we refocus the root component, focusing on invisible component first. That leads to another issue with acccessibility, but we fix it [here](#885)). A proper fix should be switching to native code, or making an API in JBR (but other vendors still be unsupported). ### Don't show input methods popup if there is no focused TextField Previously we showed a popup, even if there are no focused textfield: ![image](https://github.com/JetBrains/compose-multiplatform-core/assets/5963351/82e3543f-11f4-4013-8da5-d782b824ed2c) Now we don't show it if we isn't in a textfield. P.S. only on Windows/Linux for now, on macOs Swing seems has [a bug](JetBrains/compose-multiplatform#3839) ## Testing Tested manually on: 1. Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility enabled/disabled, ComposeWindow/ComposePanel 2. macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled 4. Linux, Chinese layout, OpenJDK 17

… issues on Windows) (#885) This fixes issues on Windows: 1. Windows pronounced "Panel unavailable" at start 2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing. 3. After merging #881, we refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown on macOs it changes one thing: 1. When we focus on Switch or Box(Modifier.clickable), now it prounonces that we in scroll area (if we in scroll area). It also pronounces it for other compnents like Button/Text ## Testing Manually with accessibility enabled in run1 (Windows, macOs)

@AiHMin

… focus (#881) ### Rerequest focus on main component when we need to type using input methods Fixes JetBrains/compose-multiplatform#2628 The issue was because of 2 things: - we used a hack to force a focused event (`component.inputContext.dispatchEvent(focusGainedEvent)` - JBR added optimization to ignore focus on the same element (thanks @AiHMin for investigation [here](JetBrains/compose-multiplatform#2628 (comment))). In Compose we have only one element. Because optimization is correct, and the hack depends on the internals, it isn't right to fix it in JBR, we should fix it in Compose. Furthermore, even without JBR changes, this hack didn't complete work - we can't for example use it for disabling input methods (see the next point). In this PR we also use a hack unfortenutely - we refocus the root component, focusing on invisible component first. That leads to another issue with acccessibility, but we fix it [here](#885)). A proper fix should be switching to native code, or making an API in JBR (but other vendors still be unsupported). ### Don't show input methods popup if there is no focused TextField Previously we showed a popup, even if there are no focused textfield: ![image](https://github.com/JetBrains/compose-multiplatform-core/assets/5963351/82e3543f-11f4-4013-8da5-d782b824ed2c) Now we don't show it if we isn't in a textfield. P.S. only on Windows/Linux for now, on macOs Swing seems has [a bug](JetBrains/compose-multiplatform#3839) ## Testing Tested manually on: 1. Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility enabled/disabled, ComposeWindow/ComposePanel 2. macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled 4. Linux, Chinese layout, OpenJDK 17

igordmn marked this pull request as draft October 19, 2023 11:34

igordmn force-pushed the igor.demin/fix-inputmethods-with-jbr-2 branch from 7a4d1b2 to 9751952 Compare October 19, 2023 22:54

igordmn force-pushed the igor.demin/fix-inputmethods-with-jbr-2 branch from 9751952 to c85c06b Compare October 19, 2023 22:59

igordmn force-pushed the igor.demin/fix-inputmethods-with-jbr-2 branch from c85c06b to 04f68f5 Compare October 19, 2023 23:52

igordmn mentioned this pull request Oct 19, 2023

Desktop. Don't use AccessibleRole.PANEL for root/unknown nodes (fixes issues on Windows) #885

Merged

igordmn changed the title ~~Desktop. Fix input methods~~ Desktop. Fix Input methods on JBR, disable input methods when we lose focus Oct 20, 2023

igordmn changed the title ~~Desktop. Fix Input methods on JBR, disable input methods when we lose focus~~ Desktop. Fix input methods on JBR, disable input methods when we lose focus Oct 20, 2023

igordmn force-pushed the igor.demin/fix-inputmethods-with-jbr-2 branch 3 times, most recently from a435648 to 7ebb7dc Compare October 20, 2023 01:11

igordmn marked this pull request as ready for review October 20, 2023 01:12

igordmn requested a review from Walingar October 20, 2023 01:13

MatkovIvan reviewed Oct 20, 2023

View reviewed changes

Walingar approved these changes Oct 20, 2023

View reviewed changes

igordmn merged commit dd86b0b into jb-main Oct 20, 2023
3 checks passed

igordmn deleted the igor.demin/fix-inputmethods-with-jbr-2 branch October 20, 2023 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop. Fix input methods on JBR, disable input methods when we lose focus #881

Desktop. Fix input methods on JBR, disable input methods when we lose focus #881

igordmn commented Oct 19, 2023 •

edited

Loading

MatkovIvan Oct 20, 2023

igordmn Oct 20, 2023

Walingar left a comment

Walingar Oct 20, 2023

igordmn Oct 20, 2023

Walingar Oct 20, 2023

igordmn Oct 20, 2023

igordmn commented Oct 20, 2023

		abstract val component: JComponent
		val invisibleComponent: Component get() = _invisibleComponent

Desktop. Fix input methods on JBR, disable input methods when we lose focus #881

Desktop. Fix input methods on JBR, disable input methods when we lose focus #881

Conversation

igordmn commented Oct 19, 2023 • edited Loading

Rerequest focus on main component when we need to type using input methods

Don't show input methods popup if there is no focused TextField

Testing

MatkovIvan Oct 20, 2023

Choose a reason for hiding this comment

igordmn Oct 20, 2023

Choose a reason for hiding this comment

Walingar left a comment

Choose a reason for hiding this comment

Walingar Oct 20, 2023

Choose a reason for hiding this comment

igordmn Oct 20, 2023

Choose a reason for hiding this comment

Walingar Oct 20, 2023

Choose a reason for hiding this comment

igordmn Oct 20, 2023

Choose a reason for hiding this comment

igordmn commented Oct 20, 2023

igordmn commented Oct 19, 2023 •

edited

Loading