Skip to content

When the user moves the caret during a composition, commit the composition #2255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import androidx.compose.ui.text.TextRange
import androidx.compose.ui.text.input.CommitTextCommand
import androidx.compose.ui.text.input.DeleteSurroundingTextCommand
import androidx.compose.ui.text.input.EditCommand
import androidx.compose.ui.text.input.FinishComposingTextCommand
import androidx.compose.ui.text.input.ImeAction
import androidx.compose.ui.text.input.ImeOptions
import androidx.compose.ui.text.input.OffsetMapping
Expand Down Expand Up @@ -150,6 +151,12 @@ internal actual fun legacyTextInputServiceAdapterAndService():
SetComposingTextCommand(text.toString(), newCursorPosition)
)
}

override fun finishComposingText() {
runOnEditCommand(
FinishComposingTextCommand()
)
}
}.block()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ private fun TextEditingScope(buffer: TextFieldBuffer) = object : TextEditingScop

buffer.cursor = newCursorInBuffer
}

override fun finishComposingText() {
buffer.commitComposition()
}
}

@OptIn(ExperimentalComposeUiApi::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,11 @@ interface TextEditingScope {
* This intends to replicate [SetComposingTextCommand].
*/
fun setComposingText(text: CharSequence, newCursorPosition: Int)

/**
* Finishes composing, leaving the text as-is.
*
* This intends to replicate [FinishComposingTextCommand].
*/
fun finishComposingText()
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package androidx.compose.ui.platform

import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.awt.toAwtRectangleRounded
import androidx.compose.ui.scene.ComposeSceneMediator
import androidx.compose.ui.text.TextRange
Expand All @@ -29,31 +30,80 @@ import java.awt.im.InputMethodRequests
import java.text.AttributedCharacterIterator
import java.text.AttributedString
import java.text.CharacterIterator
import javax.swing.SwingUtilities
import kotlin.math.max
import kotlin.math.min
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.cancel
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.launch
import kotlinx.coroutines.suspendCancellableCoroutine
import org.jetbrains.skiko.OS
import org.jetbrains.skiko.hostOs

internal class DesktopTextInputService2(
private val component: PlatformComponent
private val component: PlatformComponent,
) {

private var inputMethodSession: InputMethodSession? = null

private var receivedInputMethodEventsSinceStartInput = false
private val extraCommitEventWorkaround = ExtraCommitEventWorkaround()

private var composingText: String = ""
suspend fun startInputMethod(request: PlatformTextInputMethodRequest): Nothing {
val coroutineScope = CoroutineScope(currentCoroutineContext())
suspendCancellableCoroutine<Nothing> { continuation ->
coroutineScope.startInput(request)
continuation.invokeOnCancellation {
stopInput()
coroutineScope.cancel()
}
}
}

private fun CoroutineScope.startInput(request: PlatformTextInputMethodRequest) {
extraCommitEventWorkaround.onStartInput()

fun startInput(request: PlatformTextInputMethodRequest) {
receivedInputMethodEventsSinceStartInput = false
component.enableInput(
InputMethodSession(component, request).also {
inputMethodSession = it
}
)

// During composition, the selection should be collapsed and at the end of the composition.
// If it changes unexpectedly, finish composing.
// BTF1 commits the composition itself when the user clicks somewhere, but BTF2
// does not. This commits the composition for BTF2
launch {
snapshotFlow { request.state.selection }.collect { selection ->
val composition = request.state.composition ?: return@collect
if (!selection.collapsed || (selection.end != composition.end)) {
request.editText {
finishComposingText()
}
}
}
}

// When something commits (or otherwise ends) the composition, end it for the platform too
launch {
snapshotFlow { request.state.composition }.collect { composition ->
val composingText = inputMethodSession?.imeComposingText ?: return@collect
if (composingText.isNotEmpty() && (composition == null)) {
SwingUtilities.invokeLater {
// The event sent due to ending the composition must be ignored because the
// composition has already been committed.
// Set this in `invokeLater` to make sure we ignore the event scheduled by
// `component.endComposition()`, and not some event that has already been
// scheduled beforehand.
inputMethodSession?.ignoreNextInputMethodEvent = true
}
component.endComposition()
}
}
}
}

fun stopInput() {
private fun stopInput() {
component.disableInput()

this.inputMethodSession = null
Expand All @@ -72,35 +122,11 @@ internal class DesktopTextInputService2(
if (event.isConsumed) return
val inputMethodSession = inputMethodSession ?: return

if (commitEventWorkaroundShouldIgnoreEvent(event)) return
if (extraCommitEventWorkaround.shouldIgnoreEvent(event)) return

inputMethodSession.replaceInputMethodText(event)
inputMethodSession.inputMethodTextChanged(event)
event.consume()
}

/**
* Implements a workaround for https://youtrack.jetbrains.com/issue/CMP-7976; returns whether
* the given event should be ignored.
*
* JBR sends an extra [InputMethodEvent] when focus moves away from a text field. This event
* means to commit the current composition. Unfortunately, because we use a single actual Swing
* component (see [ComposeSceneMediator]) as a source of [InputMethodEvent]s, this event gets
* delivered to a new text session if the focus switches away to another text field.
*
* Regardless, Compose text fields commit their composition on focus loss (but not window focus
* loss) themselves, so we don't need this event.
*/
private fun commitEventWorkaroundShouldIgnoreEvent(event: InputMethodEvent): Boolean {
val isFirstEventAfterStartInput = !receivedInputMethodEventsSinceStartInput
receivedInputMethodEventsSinceStartInput = true

val currentComposingText = composingText
composingText = event.composingText

return isFirstEventAfterStartInput &&
event.committedText == currentComposingText &&
event.composingText.isEmpty()
}
}

private class InputMethodSession(
Expand All @@ -117,6 +143,12 @@ private class InputMethodSession(
private val composition: TextRange?
get() = state.composition

// The composing text from the last InputMethodEvent
var imeComposingText: String = ""
private set

var ignoreNextInputMethodEvent = false

// This is required to support input of accented characters using press-and-hold method (http://support.apple.com/kb/PH11264).
// JDK currently properly supports this functionality only for TextComponent/JTextComponent descendants.
// For our editor component we need this workaround.
Expand Down Expand Up @@ -203,10 +235,17 @@ private class InputMethodSession(
return AttributedString(committed).iterator
}

fun replaceInputMethodText(event: InputMethodEvent) {
fun inputMethodTextChanged(event: InputMethodEvent) {
val committed = event.committedText
val composing = event.composingText

imeComposingText = composing

if (ignoreNextInputMethodEvent) {
ignoreNextInputMethodEvent = false
return
}

request.editText {
if (needToDeletePreviousChar && selection.min > 0 && composing.isEmpty()) {
needToDeletePreviousChar = false
Expand Down Expand Up @@ -253,3 +292,40 @@ private fun AttributedCharacterIterator?.substringOrEmpty(
}
}
}

/**
* Implements a workaround for https://youtrack.jetbrains.com/issue/CMP-7976
*
* JBR sends an extra [InputMethodEvent] when focus moves away from a text field. This event
* means to commit the current composition. Unfortunately, because we use a single actual Swing
* component (see [ComposeSceneMediator]) as a source of [InputMethodEvent]s, this event gets
* delivered to a new text session if the focus switches away to another text field.
*
* Regardless, Compose text fields commit their composition on focus loss (but not window focus
* loss) themselves, so we don't need this event.
*/
private class ExtraCommitEventWorkaround {

private var composingText: String = ""

private var receivedInputMethodEventsSinceStartInput = false

fun onStartInput() {
receivedInputMethodEventsSinceStartInput = false
}

/**
* Returns whether the given event should be ignored.
*/
fun shouldIgnoreEvent(event: InputMethodEvent): Boolean {
val isFirstEventAfterStartInput = !receivedInputMethodEventsSinceStartInput
receivedInputMethodEventsSinceStartInput = true

val currentComposingText = composingText
composingText = event.composingText

return isFirstEventAfterStartInput &&
event.committedText == currentComposingText &&
event.composingText.isEmpty()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ internal interface PlatformComponent {

fun disableInput()

fun endComposition()

companion object {
val Empty = object : PlatformComponent {
override val locationOnScreen = Point(0, 0)
override val density = Density(1f)
override fun enableInput(inputMethodRequests: InputMethodRequests) = Unit
override fun disableInput() = Unit
override fun endComposition() = Unit
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package androidx.compose.ui.scene

import androidx.compose.ui.input.key.KeyEvent as ComposeKeyEvent
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalContext
import androidx.compose.ui.ComposeFeatureFlags
Expand All @@ -31,6 +30,7 @@ import androidx.compose.ui.focus.FocusManager
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.graphics.asComposeCanvas
import androidx.compose.ui.input.key.KeyEvent as ComposeKeyEvent
import androidx.compose.ui.input.key.internal
import androidx.compose.ui.input.key.toComposeEvent
import androidx.compose.ui.input.pointer.AwtCursor
Expand Down Expand Up @@ -90,7 +90,6 @@ import javax.swing.JComponent
import javax.swing.SwingUtilities
import kotlin.coroutines.CoroutineContext
import kotlin.math.roundToInt
import kotlinx.coroutines.suspendCancellableCoroutine
import org.jetbrains.skia.Canvas
import org.jetbrains.skiko.ClipRectangle
import org.jetbrains.skiko.ExperimentalSkikoApi
Expand Down Expand Up @@ -712,12 +711,7 @@ internal class ComposeSceneMediator(
override val textInputService = [email protected]

override suspend fun startInputMethod(request: PlatformTextInputMethodRequest): Nothing {
suspendCancellableCoroutine<Nothing> { continuation ->
textInputService2.startInput(request)
continuation.invokeOnCancellation {
textInputService2.stopInput()
}
}
textInputService2.startInputMethod(request)
}

override fun setPointerIcon(pointerIcon: PointerIcon) {
Expand Down Expand Up @@ -754,6 +748,7 @@ internal class ComposeSceneMediator(
override fun enableInput(inputMethodRequests: InputMethodRequests) {
currentInputMethodRequests = inputMethodRequests
contentComponent.enableInputMethods(true)
contentComponent.inputContext.endComposition()
// Without resetting the focus, Swing won't update the status (doesn't show/hide popup)
// enableInputMethods is design to used per-Swing component level at init stage,
// not dynamically
Expand All @@ -768,6 +763,10 @@ internal class ComposeSceneMediator(
// not dynamically
resetFocus()
}

override fun endComposition() {
contentComponent.inputContext.endComposition()
}
}

private class InvisibleComponent : Component() {
Expand Down
Loading