IssueTracker

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link:      https://android-review.googlesource.com/3328184

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling 
 
These APIs allow overscroll to have events dispatched to it by one component, and rendered in a separate component. 
 
Fixes: b/266550551 
Fixes: b/204650733 
Fixes: b/255554340 
Fixes: b/229537244 
Test: OverscrollTest 
Relnote: "Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling APIs - these APIs create a wrapped instance of the provided overscroll effect that doesn't draw / handle events respectively, which allows for rendering  overscroll in a separate component from the component that is dispatching events. For example, disabling drawing the overscroll inside a lazy list, and then drawing the overscroll separately on top / elsewhere." 
Change-Id: Idbb3d91546b49c1987a041f959bce4b2b09a9f61

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.foundation:foundation:1.8.0-alpha06
• androidx.compose.foundation:foundation-android:1.8.0-alpha06
• androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha06
• androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha06

TextField or Text? TextFields don't have any interaction with SelectionContainers, they handle selection themselves. The stack trace indicate Text or BasicText being used.

Is it a single Text within the container? or multiple?

Any other information you can provide on the layout changing? Can it happen during a selection gesture? The stack trace implies that the crash happens during the drag part of a long-press drag directly on the text (not on a handle), not the initial long-press (could be the frame right after the initial long-press though if it moves even a pixel), so I'm curious if there is any layout changing during the gesture.

Is the text scrollable?

Thanks for the information!

Any other information you can provide on the layout changing?

We are getting these crashes in an open source project and here is all the SelectionContainer uses on the project https://github.com/search?q=repo%3Apersian-calendar%2Fpersian-calendar+SelectionContainer&type=code

And here is a sample crash log, it doesn't contain where this is happening so your question regard where this happens is hard to answer given I can't reproduce the issue but users are getting them per Google Play console, if you have access to Google Play Console's crash logs please have a look at this package crashes: com.byagowi.persiancalendar this is the third crash there of the release v9.1.1. The app is free from Internet and doesn't even get Internet permission in the manifest so should be safe enough for your testing also, and here is the source https://github.com/persian-calendar/persian-calendar/tree/v9.1.1 (the latest release on Google Play tag)

Exception java.lang.IllegalStateException: SelectionLayout must not be empty.
  at androidx.compose.foundation.text.selection.SelectionLayoutBuilder.build (SelectionLayoutBuilder.java:726)
  at androidx.compose.foundation.text.selection.SelectionManager.getSelectionLayout-Wko1d7g (SelectionManager.kt:726)
  at androidx.compose.foundation.text.selection.SelectionManager.updateSelection-jyLRC_s$foundation_release (SelectionManager.kt:726)
  at androidx.compose.foundation.text.selection.SelectionManager.updateSelection-qNKwrvQ$foundation_release (SelectionManager.kt:44)
  at androidx.compose.foundation.text.selection.SelectionManager$4.invoke-pGV3PM0 (SelectionManager.kt:44)
  at androidx.compose.foundation.text.selection.SelectionManager$4.invoke (SelectionManager.kt:44)
  at androidx.compose.foundation.text.selection.SelectionRegistrarImpl.notifySelectionUpdate-njBpvok (SelectionRegistrarImpl.kt:23)
  at androidx.compose.foundation.text.modifiers.SelectionControllerKt$makeSelectionModifier$longPressDragObserver$1.e (SelectionController.kt:53)
  at androidx.compose.foundation.text.selection.SelectionGesturesKt$touchSelection$2.invoke (SelectionGestures.kt:17)
  at androidx.compose.foundation.text.selection.SelectionGesturesKt$touchSelection$2.invoke (SelectionGestures.kt:17)
  at androidx.compose.foundation.gestures.DragGestureDetectorKt.drag-jO51t88 (DragGestureDetector.kt:86)
  at androidx.compose.foundation.gestures.DragGestureDetectorKt$drag$1.invokeSuspend (DragGestureDetector.kt:1)
  at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:1)

Were there crashes before 9.1.1 release?

If there weren't, from what I can tell, compose foundation (the containing package) went from 1.6.0-rc01 to 1.6.1, which could help in trying to find what changed.

Tried to reproduce by messing with the size of text and visibility in animation (to make the size 0) but was unable to repro even when dragging on a zero size text.

Were there crashes before 9.1.1 release?

The app had three releases with Compose, v9.0.0, v9.1.0 and v9.1.1 and they all had these crashes per Google Play console (before these versions the app was Compose-free)

Our code is pretty simple:

SelectionContainer(
    modifier = Modifier.weight(1f)
) {
    Text(text = description)
}

There are some Rows/Columns around and the Compose code is set to a androidx.compose.ui.platform.ComposeView which is collapsed to a height of 0, but nothing special. It's tightly coupled with our project so it's not easy to show a small example code/project.

What I noticed is that this issue only happened on Samsung devices (Galaxy S9+, S20, S20 FE, S21+ 5G) so far. Maybe there is some specific implementation detail they are doing differently. But maybe that's also just coincidence.

Still no luck. I would be surprised if it was a samsung device thing, this code is pretty solidly in Compose.

I tried more ways of getting 0 size layouts and none of them worked. Also tried getting a lazy list to forget a text that was under a gesture, but it cancels the gesture correctly.

Some more notes about these code paths, In order to get to the point of "empty layout builder":

• All Texts must be ruled out in one way or another in the selection container logic. Most examples here only have a single text in a selection container, so only the one has to be excluded from the container for it to be empty.
• One possibility is that there are no selectables in the first place.
  • In order for the drag to happen, it implies that there is some BasicText available for it to happen on, so this seems unlikely. Could this run after the basic text is unsubscribed from the selection registrar? also seems unlikely.
• Layout coordinates not being attached or not existing at all is another reason for one to be excluded, but this is checked much earlier in the stack trace, so is also unlikely.
• TextLayoutResult being null is the last reason for being excluded, but it is set at the first measure of the composable, and the gesture happening after the gesture is initiated and before the text is measured seems very unlikely.

All that being said, if anyone can repro this or narrow down the conditions required to repro it, please comment :)

It doesn't only occur on Samsung devices. See the attached screenshot.

Thanks for the additional info folks.

For the person with 1/10 attempts end in a crash, can you provide a sample to reproduce this?

sure, here is a recording showing taps while trying to select some text, ending with a crash. Code sample:

LazyColumn(
    contentPadding = PaddingValues(top = 4.dp, bottom = 40.dp),
    state = lazyListState
) {
    items(items = allWords) { word ->
        Column(
            modifier = Modifier
                .wrapContentHeight()
                .fillMaxWidth(),
            horizontalAlignment = Alignment.CenterHorizontally
        ) {
            SelectionContainer {
                Column(
                    modifier = Modifier
                        .wrapContentHeight()
                        .fillMaxWidth()
                        .padding(horizontal = 4.dp),
                    horizontalAlignment = Alignment.CenterHorizontally
                ) {
                    Text(
                        text = "${word.original.orEmpty()}:",
                        textAlign = TextAlign.Center
                    )
                    Spacer(modifier = Modifier.size(4.dp))
                    Text(
                        text = word.translation.orEmpty(),
                        textAlign = TextAlign.Center,
                        color = Color.Green
                    )
                    Spacer(modifier = Modifier.size(4.dp))
                    Divider()
                }
            }
        }
    }
}

video from Samsung Galaxy S20

Can you share what compose versions you are using and what Android version?

For my own reference, code translated to foundation only:

Edit: Adding borders around text/selection container.

import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.wrapContentHeight
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.foundation.text.BasicText
import androidx.compose.foundation.text.selection.SelectionContainer
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp

data class Word(val original: String, val translation: String)

val word = Word("Original", "Translation")
val words = List(40) { word }

@Composable
fun SelectLayoutCrashRepro() {
    Row(Modifier.fillMaxSize()) {
        Box(Modifier.weight(1f)) {
            SelectLayoutCrashReproColumn()
        }
        Spacer(modifier = Modifier.size(16.dp))
        Box(Modifier.weight(1f)) {
            SelectLayoutCrashReproColumn()
        }
    }
}

@Composable
fun SelectLayoutCrashReproColumn() {
    val lazyListState = rememberLazyListState()
    LazyColumn(
        contentPadding = PaddingValues(top = 4.dp, bottom = 40.dp),
        state = lazyListState
    ) {
        items(items = words) { word ->
            Column(
                modifier = Modifier
                    .wrapContentHeight()
                    .fillMaxWidth(),
                horizontalAlignment = Alignment.CenterHorizontally
            ) {
                SelectionContainer {
                    Column(
                        modifier = Modifier
                            .wrapContentHeight()
                            .fillMaxWidth()
                            .padding(horizontal = 4.dp)
                            .border(Dp.Hairline, Color.Red),
                        horizontalAlignment = Alignment.CenterHorizontally
                    ) {
                        BasicText(
                            text = "${word.original}:",
                            style = TextStyle(textAlign = TextAlign.Center),
                            modifier = Modifier.border(Dp.Hairline, Color.Blue),

                            )
                        Spacer(modifier = Modifier.size(4.dp))
                        BasicText(
                            text = word.translation,
                            style = TextStyle(
                                textAlign = TextAlign.Center,
                                color = Color.Green,
                            ),
                            modifier = Modifier.border(Dp.Hairline, Color.Blue),
                        )
                        Spacer(modifier = Modifier.size(4.dp))
                        Spacer(
                            modifier = Modifier
                                .fillMaxWidth()
                                .height(1.dp)
                                .background(Color.LightGray)
                        )
                    }
                }
            }
        }
    }
}

Thanks, I was able to reproduce it, but it is pretty picky to do so.

1. Start touch selection via long press
2. Drag the selection out of the text bounds before the selection updates for the first time
3. Some logic that expects a previous selection is unable to resolve correctly, and throws the error.

At a high level, the previous selection provides two anchor points, start and end, which determine where the selection is. The end anchor is being moved as part of the drag, so we use the start anchor to determine which texts to include in the selection. Since the previous selection is null at this point, our code sees no texts to include in the selection, hence why we end up with an error indicating that the selection layout is empty. Usually when the previous selection is null, it is because we are currently in an onStart code path where it makes sense that there cannot be a previous selection. The onStart logic coerces the touch into the bounds of the text, which is why this error never happens in onStart, since step 2 of the repro instructions - drag out of text bounds - can't happen. This is what was fixed in b/299787906.

The new problem seems to be the assumption that previousSelection will be set to a non-null value as part of the gesture onStart. It seems that it may be possible that the onDrag occurs before the start has finished updating previousSelection, leaving it as null for the onDrag.

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.foundation:foundation:1.6.4
• androidx.compose.foundation:foundation-android:1.6.4
• androidx.compose.foundation:foundation-desktop:1.6.4
• androidx.compose.foundation:foundation:1.7.0-alpha05
• androidx.compose.foundation:foundation-android:1.7.0-alpha05
• androidx.compose.foundation:foundation-desktop:1.7.0-alpha05

If you find more issues in versions greater than or equal to the above, please create a new bug that may contain references to this one.