Slider cause app crash when density change [231707291]

Fixed

Bug

Status Update

No update yet.

Description

ra...@gmail.com

created issue #1

May 7, 2022 02:24PM

Jetpack Compose version: 1.2.0-alpha07 Jetpack Compose component used: material3

Use Slider with zero width constrants, then change the density, the Slider might causes app crash:

java.lang.IllegalArgumentException: Padding must be non-negative
        at androidx.compose.foundation.layout.PaddingModifier.<init>(Padding.kt:346)
        at androidx.compose.foundation.layout.PaddingModifier.<init>(Unknown Source:0)
        at androidx.compose.foundation.layout.PaddingKt.padding-qDBjuR0(Padding.kt:56)
        at androidx.compose.foundation.layout.PaddingKt.padding-qDBjuR0$default(Padding.kt:50)
        at androidx.compose.material3.SliderKt.SliderThumb-i-S6nvU(Slider.kt:385)
        at androidx.compose.material3.SliderKt.SliderImpl(Slider.kt:371)
        at androidx.compose.material3.SliderKt.access$SliderImpl(Slider.kt:1)
        at androidx.compose.material3.SliderKt$Slider$3.invoke(Slider.kt:207)
        at androidx.compose.material3.SliderKt$Slider$3.invoke(Slider.kt:155)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116)
        at androidx.compose.runtime.internal.ComposableLambdaImpl$invoke$1.invoke(ComposableLambda.jvm.kt:127)
        at androidx.compose.runtime.internal.ComposableLambdaImpl$invoke$1.invoke(ComposableLambda.jvm.kt:127)
        at androidx.compose.runtime.RecomposeScopeImpl.compose(RecomposeScopeImpl.kt:157)
        at androidx.compose.runtime.ComposerImpl.recomposeToGroupEnd(Composer.kt:2287)
        at androidx.compose.runtime.ComposerImpl.skipCurrentGroup(Composer.kt:2547)
        at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3076)
        at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3054)
        at androidx.compose.runtime.SnapshotStateKt__DerivedStateKt.observeDerivedStateRecalculations(DerivedState.kt:252)
        at androidx.compose.runtime.SnapshotStateKt.observeDerivedStateRecalculations(Unknown Source:1)
        at androidx.compose.runtime.ComposerImpl.doCompose(Composer.kt:3054)
        at androidx.compose.runtime.ComposerImpl.recompose$runtime_release(Composer.kt:3020)
        at androidx.compose.runtime.CompositionImpl.recompose(Composition.kt:722)
        at androidx.compose.runtime.Recomposer.performRecompose(Recomposer.kt:876)
        at androidx.compose.runtime.Recomposer.access$performRecompose(Recomposer.kt:107)
        at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:485)
        at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:454)
        at androidx.compose.ui.platform.AndroidUiFrameClock$withFrameNanos$2$callback$1.doFrame(AndroidUiFrameClock.android.kt:34)
        at androidx.compose.ui.platform.AndroidUiDispatcher.performFrameDispatch(AndroidUiDispatcher.android.kt:109)
        at androidx.compose.ui.platform.AndroidUiDispatcher.access$performFrameDispatch(AndroidUiDispatcher.android.kt:41)
        at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.doFrame(AndroidUiDispatcher.android.kt:69)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:964)
        at android.view.Choreographer.doCallbacks(Choreographer.java:790)
        at android.view.Choreographer.doFrame(Choreographer.java:721)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:951)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Here is a simple code to reproduce that:

val currentDensity = LocalDensity.current
            var densityMultiplier by remember {
                mutableStateOf(1f)
            }
            CompositionLocalProvider(
                LocalDensity provides Density(currentDensity.density * densityMultiplier)
            ) {
                Box(modifier = Modifier.requiredWidth(0.dp)) {
                    Slider(
                        value = densityMultiplier,
                        onValueChange = { densityMultiplier = it }
                    )
                }
            }

Just drag the slider, it will crash very soon

Comments

ra...@gmail.com <ra...@gmail.com> #2May 7, 2022 02:31PM

Wow, it seems like a very important bug to fix.

Doing some investigations...

It works fine with CoreTextField as well as BaseTextField.

sg...@google.com <sg...@google.com> May 7, 2022 06:51PM

Reassigned to ma...@google.com.

ma...@google.com <ma...@google.com> #3May 9, 2022 03:19PM

Status: Won't Fix (Intended Behavior)

Seems to be a duplicate of b/163834907

ra...@gmail.com <ra...@gmail.com> #4May 9, 2022 11:58PM

It works fine with Material TextFields (both filled and outline) if you use TextFieldValue based overloads.

It doesn't work when you use String based overloads however (both Outline and Filled). I believe this is related to the one-frame lag as we host TextFieldValue internally and proxy only string.

ra...@gmail.com <ra...@gmail.com> #5May 10, 2022 12:14AM

Chatted with Anastasia.

We have this code

// "value: String" comes as a param in TextField
TextFieldImpl(
        type = TextFieldType.Outlined,
        value = textFieldValue,
        onValueChange = {
            selection = it.selection
            composition = it.composition
            if (value != it.text) {
                onValueChange(it.text)
            }
        },

and this value is always "" for me, as it's captured my lambda when lambda was created and this lambda is saved and reused by CoreTextField.

Anyway, two things we can do:

Remove this if and invoke onValueChange everytime, even on the selection change (which we don't care about). This will allow us to workaround quickly and not to let it slip through.
Find and implement the proper fix in CoreTextField

Let me know what you think

ma...@google.com <ma...@google.com> #6May 10, 2022 07:16PM

Status: Assigned (reopened)

Updated the details here b/163834907.

ra...@gmail.com <ra...@gmail.com> #7May 10, 2022 11:45PM

Just a note that this issue affects String overrides of both material text fields as well as any custom String override text fields that developer might create on top of CoreTextField/BaseTextField.

ap...@google.com <ap...@google.com> #8May 10, 2022 11:54PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 1717097cfa889c564450626af6d5dcc405581d79
Author: Anastasia Soboleva <soboleva@google.com>
Date: Mon Aug 24 16:38:59 2020

Fix lambda value being captured for string override text fields

Fixes: 163834907
Fixes: 163808219
Fixes: 165956313
Test: new test in TextFieldTest#stringOverrideTextField_canDeleteLastSymbol
Test: foundation and material tests passed
Change-Id: I9932a2bddc3859a2a35a54e0b6d7cb525853d3ac

M compose/foundation/foundation-text/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TextFieldTest.kt

https://android-review.googlesource.com/1408268