Skip to content
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

In Compose version 1.6.1, the optional text component causes an exception when clicking on no text area. #4555

Closed
ParrotCoder opened this issue Mar 29, 2024 · 2 comments · Fixed by JetBrains/compose-multiplatform-core#1230
Assignees
Labels

Comments

@ParrotCoder
Copy link

ParrotCoder commented Mar 29, 2024

Describe the bug
In Compose 1.6.1 version, declare the Text component in SelectionContainer and set the size of the Text component. When clicking an area without text, an error occurs: "java.lang.IllegalStateException: SelectionLayout must not be empty."
The same code is in Compose 1.5.10 Version will not cause errors

Affected platforms

  • Desktop (Windows, Linux, macOS)

Versions

  • Kotlin version*: 1.9.0
  • Compose Multiplatform version*: 1.6.1
  • OS version(s)* (required for Desktop and iOS issues): Windows10 Pro 22H2 19045.3324
  • OS architecture (x86 or arm64): x86
  • JDK (for desktop issues): jbr-17

To Reproduce

  1. Run the following code
  2. Click on the area where the text component has no text
  3. See error "SelectionLayout must not be empty."
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.text.selection.SelectionContainer
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.WindowPosition
import androidx.compose.ui.window.application
import androidx.compose.ui.window.rememberWindowState

fun main() = application {
	Window(
		onCloseRequest = ::exitApplication,
		state = rememberWindowState(
			position = WindowPosition(Alignment.Center)
		)
	) {
		App()
	}
}

@Composable
private fun App() {
	Box(
		modifier = Modifier.padding(10.dp)
	) {
		SelectionContainer {
			val text by remember { mutableStateOf("Text Content\r\nText Content2") }
			Text(
				text = text,
				modifier = Modifier
					.fillMaxSize()
					.background(Color.LightGray)
			)
		}
	}
}

Expected behavior
No errors should occur

Screenshots
image

@ParrotCoder ParrotCoder added bug Something isn't working submitted labels Mar 29, 2024
@m-sasha m-sasha self-assigned this Mar 29, 2024
@m-sasha
Copy link
Contributor

m-sasha commented Mar 29, 2024

The difference in our version that causes this is in MultiWidgetSelectionDelegate.kt:

internal fun SelectionLayoutBuilder.appendSelectableInfo(
    textLayoutResult: TextLayoutResult,
    localPosition: Offset,
    previousHandlePosition: Offset,
    selectableId: Long,
) {
    val bounds = Rect(
        0.0f,
        0.0f,
        textLayoutResult.multiParagraph.width.toFloat(),
        textLayoutResult.multiParagraph.height.toFloat()
    )
    ...
}

We use textLayoutResult.multiParagraph for the bounds size, while Android uses textLayoutResult.size. In the reproducer, textLayoutResult.size includes the entire Text (height is large), while textLayoutResult.multiParagraph only includes (vertically, at least) the area with the visual text itself (height is small; although width is large, not sure why).

Due to this, the code that follows it immediately behaves differently. currentYDirection is AFTER instead of ON, and then isSelected() a few lines below returns false, which causes the function to return without calling appendInfo. Then SelectionManager.getSelectionLayout calls SelectionLayoutBuilder.build, which throws an exception since infoList is empty.

I can't say whether the bug is the use of textLayoutResult.multiParagraph itself, or perhaps SelectionManager.getSelectionLayout should not assume that appendSelectableInfoToBuilder succeeded, or maybe appendSelectableInfo should behave differently when currentYDirection is AFTER.

It makes sense that even if dragging begins outside of the bounds of the visible text, it should still start selecting, so that when the cursor is dragged inside the visible text, the selection works. So I think appendSelectableInfo should call appendInfo in this case.

The change was introduced in 2021 in commit 37564c1fd132a637afd8d31466e52c290c1bfb17 and was meant to fix several bugs. I've changed the code to use, as in Android, textLayoutResult.size, and checked all the cases the commit refers to. All of them work correctly.

Because of this, and because it's much easier to revert to the Android version than to understand and agree on how the surrounding code should behave, then change and upstream it, I'd recommend reverting to the Android version.

@igordmn ?

@okushnikov
Copy link

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants