diff --git a/CHANGELOG.md b/CHANGELOG.md index 57d640f385a..0cfda7df07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Session Replay: Fix Compose text masking mismatch with weighted text ([#5218](https://github.com/getsentry/sentry-java/pull/5218)) + ### Features - Android: Add `beforeErrorSampling` callback to Session Replay ([#5214](https://github.com/getsentry/sentry-java/pull/5214)) diff --git a/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png b/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png index 31ba5f581bc..aa1ec41ee06 100644 Binary files a/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png and b/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png differ diff --git a/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png b/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png index 952fd6ba634..6ffbe1e07f5 100644 Binary files a/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png and b/sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png differ diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt index 1d779d389bf..cd9e3dd208a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt @@ -13,38 +13,39 @@ import androidx.compose.ui.node.LayoutNode import androidx.compose.ui.text.TextLayoutResult import kotlin.math.roundToInt -internal class ComposeTextLayout( - internal val layout: TextLayoutResult, - private val hasFillModifier: Boolean, -) : TextLayout { +internal class ComposeTextLayout(internal val layout: TextLayoutResult) : TextLayout { override val lineCount: Int get() = layout.lineCount override val dominantTextColor: Int? get() = null - override fun getPrimaryHorizontal(line: Int, offset: Int): Float { - val horizontalPos = layout.getHorizontalPosition(offset, usePrimaryDirection = true) - // when there's no `fill` modifier on a Text composable, compose still thinks that there's - // one and wrongly calculates horizontal position relative to node's start, not text's start - // for some reason. This is only the case for single-line text (multiline works fien). - // So we subtract line's left to get the correct position - return if (!hasFillModifier && lineCount == 1) { - horizontalPos - layout.getLineLeft(line) - } else { - horizontalPos + /** + * The paragraph may be laid out with a wider width (constraint maxWidth) than the actual node + * (layout result size). When that happens, getLineLeft/getLineRight return positions in the + * paragraph coordinate system, which don't match the node's bounds. In that case, text alignment + * has no visible effect, so we fall back to using line width starting from x=0. + */ + private val paragraphWidthExceedsNode: Boolean + get() = layout.multiParagraph.width > layout.size.width + + override fun getLineLeft(line: Int): Float { + if (paragraphWidthExceedsNode) { + return 0f } + return layout.getLineLeft(line) } - override fun getEllipsisCount(line: Int): Int = if (layout.isLineEllipsized(line)) 1 else 0 - - override fun getLineVisibleEnd(line: Int): Int = layout.getLineEnd(line, visibleEnd = true) + override fun getLineRight(line: Int): Float { + if (paragraphWidthExceedsNode) { + return layout.multiParagraph.getLineWidth(line) + } + return layout.getLineRight(line) + } override fun getLineTop(line: Int): Int = layout.getLineTop(line).roundToInt() override fun getLineBottom(line: Int): Int = layout.getLineBottom(line).roundToInt() - - override fun getLineStart(line: Int): Int = layout.getLineStart(line) } // TODO: probably most of the below we can do via bytecode instrumentation and speed up at runtime @@ -92,8 +93,6 @@ internal fun Painter.isMaskable(): Boolean { !className.contains("Brush") } -internal data class TextAttributes(val color: Color?, val hasFillModifier: Boolean) - /** * This method is necessary to mask text in Compose. * @@ -101,37 +100,24 @@ internal data class TextAttributes(val color: Color?, val hasFillModifier: Boole * string in their name, e.g. TextStringSimpleElement or TextAnnotatedStringElement. We then get the * color from the modifier, to be able to mask it with the correct color. * - * We also look up for classes that have a [Fill] modifier, usually they all have a `Fill` string in - * their name, e.g. FillElement. This is necessary to workaround a Compose bug where single-line - * text composable without a `fill` modifier still thinks that there's one and wrongly calculates - * horizontal position. - * * We also add special proguard rules to keep the `Text` class names and their `color` member. */ -internal fun LayoutNode.findTextAttributes(): TextAttributes { +internal fun LayoutNode.findTextColor(): Color? { val modifierInfos = getModifierInfo() - var color: Color? = null - var hasFillModifier = false for (index in modifierInfos.indices) { val modifier = modifierInfos[index].modifier val modifierClassName = modifier::class.java.name if (modifierClassName.contains("Text")) { - color = - try { - (modifier::class - .java - .getDeclaredField("color") - .apply { isAccessible = true } - .get(modifier) as? ColorProducer) - ?.invoke() - } catch (e: Throwable) { - null - } - } else if (modifierClassName.contains("Fill")) { - hasFillModifier = true + return try { + (modifier::class.java.getDeclaredField("color").apply { isAccessible = true }.get(modifier) + as? ColorProducer) + ?.invoke() + } catch (e: Throwable) { + null + } } } - return TextAttributes(color, hasFillModifier) + return null } /** diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/TextLayout.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/TextLayout.kt index 9b974efceaa..e62584bda06 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/TextLayout.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/TextLayout.kt @@ -13,15 +13,11 @@ internal interface TextLayout { */ val dominantTextColor: Int? - fun getPrimaryHorizontal(line: Int, offset: Int): Float + fun getLineLeft(line: Int): Float - fun getEllipsisCount(line: Int): Int - - fun getLineVisibleEnd(line: Int): Int + fun getLineRight(line: Int): Float fun getLineTop(line: Int): Int fun getLineBottom(line: Int): Int - - fun getLineStart(line: Int): Int } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt index 80ca1beee4e..b7f09513930 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Views.kt @@ -128,21 +128,14 @@ internal fun TextLayout?.getVisibleRects( val rects = mutableListOf() for (i in 0 until lineCount) { - val lineStart = getPrimaryHorizontal(i, getLineStart(i)).toInt() - val ellipsisCount = getEllipsisCount(i) - val lineVisibleEnd = getLineVisibleEnd(i) - var lineEnd = - getPrimaryHorizontal(i, lineVisibleEnd - ellipsisCount + if (ellipsisCount > 0) 1 else 0) - .toInt() - if (lineEnd == 0 && lineVisibleEnd > 0) { - // looks like the case for when emojis are present in text - lineEnd = getPrimaryHorizontal(i, lineVisibleEnd - 1).toInt() + 1 - } + val lineLeft = getLineLeft(i).toInt() + val lineRight = getLineRight(i).toInt() val lineTop = getLineTop(i) val lineBottom = getLineBottom(i) val rect = Rect() - rect.left = globalRect.left + paddingLeft + lineStart - rect.right = rect.left + (lineEnd - lineStart) + + rect.left = globalRect.left + paddingLeft + lineLeft + rect.right = globalRect.left + paddingLeft + lineRight rect.top = globalRect.top + paddingTop + lineTop rect.bottom = rect.top + (lineBottom - lineTop) @@ -197,18 +190,13 @@ internal class AndroidTextLayout(private val layout: Layout) : TextLayout { return dominantColor?.toOpaque() } - override fun getPrimaryHorizontal(line: Int, offset: Int): Float = - layout.getPrimaryHorizontal(offset) + override fun getLineLeft(line: Int): Float = layout.getLineLeft(line) - override fun getEllipsisCount(line: Int): Int = layout.getEllipsisCount(line) - - override fun getLineVisibleEnd(line: Int): Int = layout.getLineVisibleEnd(line) + override fun getLineRight(line: Int): Float = layout.getLineRight(line) override fun getLineTop(line: Int): Int = layout.getLineTop(line) override fun getLineBottom(line: Int): Int = layout.getLineBottom(line) - - override fun getLineStart(line: Int): Int = layout.getLineStart(line) } internal fun View?.addOnDrawListenerSafe(listener: ViewTreeObserver.OnDrawListener) { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt index f421ff9ad07..ec01d28d4fb 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt @@ -24,7 +24,7 @@ import io.sentry.android.replay.SentryReplayModifiers import io.sentry.android.replay.util.ComposeTextLayout import io.sentry.android.replay.util.boundsInWindow import io.sentry.android.replay.util.findPainter -import io.sentry.android.replay.util.findTextAttributes +import io.sentry.android.replay.util.findTextColor import io.sentry.android.replay.util.isMaskable import io.sentry.android.replay.util.toOpaque import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.GenericViewHierarchyNode @@ -189,11 +189,10 @@ internal object ComposeViewHierarchyNode { ?.action ?.invoke(textLayoutResults) - val (color, hasFillModifier) = node.findTextAttributes() val textLayoutResult = textLayoutResults.firstOrNull() var textColor = textLayoutResult?.layoutInput?.style?.color if (textColor?.isUnspecified == true) { - textColor = color + textColor = node.findTextColor() } val isLaidOut = textLayoutResult?.layoutInput?.style?.fontSize != TextUnit.Unspecified // TODO: support editable text (currently there's a way to get @Composable's padding only @@ -202,7 +201,7 @@ internal object ComposeViewHierarchyNode { TextViewHierarchyNode( layout = if (textLayoutResult != null && !isEditable && isLaidOut) { - ComposeTextLayout(textLayoutResult, hasFillModifier) + ComposeTextLayout(textLayoutResult) } else { null },