Skip to content

Commit 0f34885

Browse files
markushimarkushi
authored
Merge 8cd317c into e2dce0b 
2 parents e2dce0b + 8cd317c commit 0f34885

12 files changed

Lines changed: 302 additions & 72 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixes
6+
7+
- Session Replay: Fix Compose text masking mismatch with weighted text ([#5218](https://github.com/getsentry/sentry-java/pull/5218))
8+
59
### Features
610

711
- Android: Add `beforeErrorSampling` callback to Session Replay ([#5214](https://github.com/getsentry/sentry-java/pull/5214))

‎sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,27 @@ import android.graphics.Color
99
import android.graphics.drawable.Drawable
1010
import android.os.Bundle
1111
import android.os.Looper
12+
import android.text.TextUtils
1213
import android.view.View
1314
import android.widget.ImageView
1415
import android.widget.LinearLayout
1516
import android.widget.LinearLayout.LayoutParams
1617
import android.widget.RadioButton
1718
import android.widget.TextView
19+
import androidx.activity.ComponentActivity
20+
import androidx.activity.compose.setContent
21+
import androidx.compose.foundation.background
22+
import androidx.compose.foundation.layout.Arrangement
23+
import androidx.compose.foundation.layout.Column
24+
import androidx.compose.foundation.layout.Row
25+
import androidx.compose.foundation.layout.fillMaxWidth
26+
import androidx.compose.foundation.layout.padding
27+
import androidx.compose.material3.Text
28+
import androidx.compose.ui.Modifier
29+
import androidx.compose.ui.text.style.TextAlign
30+
import androidx.compose.ui.text.style.TextOverflow
31+
import androidx.compose.ui.unit.dp
32+
import androidx.compose.ui.unit.sp
1833
import androidx.test.ext.junit.runners.AndroidJUnit4
1934
import com.dropbox.differ.Color as DifferColor
2035
import com.dropbox.differ.Image
@@ -410,6 +425,48 @@ class ScreenshotEventProcessorTest {
410425
assertNotNull(bytes)
411426
}
412427

428+
@Test
429+
fun `snapshot - screenshot with ellipsized text no masking`() {
430+
fixture.activity = buildActivity(EllipsizedTextActivity::class.java, null).setup().get()
431+
val bytes =
432+
processEventForSnapshots(
433+
"screenshot_mask_ellipsized_view_unmasked",
434+
isReplayAvailable = false,
435+
)
436+
assertNotNull(bytes)
437+
}
438+
439+
@Test
440+
fun `snapshot - screenshot with ellipsized text masking`() {
441+
fixture.activity = buildActivity(EllipsizedTextActivity::class.java, null).setup().get()
442+
val bytes =
443+
processEventForSnapshots("screenshot_mask_ellipsized_view_masked") {
444+
it.screenshot.setMaskAllText(true)
445+
}
446+
assertNotNull(bytes)
447+
}
448+
449+
@Test
450+
fun `snapshot - compose text no masking`() {
451+
fixture.activity = buildActivity(ComposeTextActivity::class.java, null).setup().get()
452+
val bytes =
453+
processEventForSnapshots(
454+
"screenshot_mask_ellipsized_compose_unmasked",
455+
isReplayAvailable = false,
456+
)
457+
assertNotNull(bytes)
458+
}
459+
460+
@Test
461+
fun `snapshot - compose text with masking`() {
462+
fixture.activity = buildActivity(ComposeTextActivity::class.java, null).setup().get()
463+
val bytes =
464+
processEventForSnapshots("screenshot_mask_ellipsized_compose_masked") {
465+
it.screenshot.setMaskAllText(true)
466+
}
467+
assertNotNull(bytes)
468+
}
469+
413470
// endregion
414471

415472
private fun getEvent(): SentryEvent = SentryEvent(Throwable("Throwable"))
@@ -484,6 +541,189 @@ private class CustomView(context: Context) : View(context) {
484541
}
485542
}
486543

544+
private class EllipsizedTextActivity : Activity() {
545+
546+
override fun onCreate(savedInstanceState: Bundle?) {
547+
super.onCreate(savedInstanceState)
548+
val longText = "This is a very long text that should be ellipsized when it does not fit"
549+
550+
val linearLayout =
551+
LinearLayout(this).apply {
552+
setBackgroundColor(Color.WHITE)
553+
orientation = LinearLayout.VERTICAL
554+
layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT)
555+
setPadding(10, 10, 10, 10)
556+
}
557+
558+
// Ellipsize end
559+
linearLayout.addView(
560+
TextView(this).apply {
561+
text = longText
562+
setTextColor(Color.BLACK)
563+
textSize = 16f
564+
maxLines = 1
565+
ellipsize = TextUtils.TruncateAt.END
566+
setBackgroundColor(Color.LTGRAY)
567+
layoutParams =
568+
LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT).apply {
569+
setMargins(0, 8, 0, 0)
570+
}
571+
}
572+
)
573+
574+
// Ellipsize middle
575+
linearLayout.addView(
576+
TextView(this).apply {
577+
text = longText
578+
setTextColor(Color.BLACK)
579+
textSize = 16f
580+
maxLines = 1
581+
ellipsize = TextUtils.TruncateAt.MIDDLE
582+
setBackgroundColor(Color.LTGRAY)
583+
layoutParams =
584+
LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT).apply {
585+
setMargins(0, 8, 0, 0)
586+
}
587+
}
588+
)
589+
590+
// Ellipsize start
591+
linearLayout.addView(
592+
TextView(this).apply {
593+
text = longText
594+
setTextColor(Color.BLACK)
595+
textSize = 16f
596+
maxLines = 1
597+
ellipsize = TextUtils.TruncateAt.START
598+
setBackgroundColor(Color.LTGRAY)
599+
layoutParams =
600+
LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT).apply {
601+
setMargins(0, 8, 0, 0)
602+
}
603+
}
604+
)
605+
606+
// Non-ellipsized text for comparison
607+
linearLayout.addView(
608+
TextView(this).apply {
609+
text = "Short text"
610+
setTextColor(Color.BLACK)
611+
textSize = 16f
612+
setBackgroundColor(Color.LTGRAY)
613+
layoutParams =
614+
LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT).apply {
615+
setMargins(0, 8, 0, 0)
616+
}
617+
}
618+
)
619+
620+
setContentView(linearLayout)
621+
}
622+
}
623+
624+
private class ComposeTextActivity : ComponentActivity() {
625+
626+
override fun onCreate(savedInstanceState: Bundle?) {
627+
super.onCreate(savedInstanceState)
628+
val longText = "This is a very long text that should be ellipsized when it does not fit in view"
629+
630+
setContent {
631+
Column(
632+
modifier =
633+
Modifier.fillMaxWidth()
634+
.background(androidx.compose.ui.graphics.Color.White)
635+
.padding(10.dp),
636+
verticalArrangement = Arrangement.spacedBy(8.dp),
637+
) {
638+
// Ellipsis overflow
639+
Text(
640+
longText,
641+
maxLines = 1,
642+
overflow = TextOverflow.Ellipsis,
643+
fontSize = 16.sp,
644+
modifier =
645+
Modifier.fillMaxWidth().background(androidx.compose.ui.graphics.Color.LightGray),
646+
)
647+
648+
// Text with textAlign center
649+
Text(
650+
"Centered text",
651+
textAlign = TextAlign.Center,
652+
fontSize = 16.sp,
653+
modifier =
654+
Modifier.fillMaxWidth().background(androidx.compose.ui.graphics.Color.LightGray),
655+
)
656+
657+
// Text with textAlign end
658+
Text(
659+
"End-aligned text",
660+
textAlign = TextAlign.End,
661+
fontSize = 16.sp,
662+
modifier =
663+
Modifier.fillMaxWidth().background(androidx.compose.ui.graphics.Color.LightGray),
664+
)
665+
666+
// Ellipsis with textAlign center
667+
Text(
668+
longText,
669+
maxLines = 1,
670+
overflow = TextOverflow.Ellipsis,
671+
textAlign = TextAlign.Center,
672+
fontSize = 16.sp,
673+
modifier =
674+
Modifier.fillMaxWidth().background(androidx.compose.ui.graphics.Color.LightGray),
675+
)
676+
677+
// Weighted row with text
678+
Row(
679+
modifier = Modifier.fillMaxWidth(),
680+
horizontalArrangement = Arrangement.spacedBy(8.dp),
681+
) {
682+
Text(
683+
"Weight 1",
684+
fontSize = 16.sp,
685+
modifier = Modifier.weight(1f).background(androidx.compose.ui.graphics.Color.LightGray),
686+
)
687+
Text(
688+
"Weight 1",
689+
fontSize = 16.sp,
690+
modifier = Modifier.weight(1f).background(androidx.compose.ui.graphics.Color.LightGray),
691+
)
692+
}
693+
694+
// Weighted row with ellipsized text
695+
Row(
696+
modifier = Modifier.fillMaxWidth(),
697+
horizontalArrangement = Arrangement.spacedBy(8.dp),
698+
) {
699+
Text(
700+
longText,
701+
maxLines = 1,
702+
overflow = TextOverflow.Ellipsis,
703+
fontSize = 16.sp,
704+
modifier = Modifier.weight(1f).background(androidx.compose.ui.graphics.Color.LightGray),
705+
)
706+
Text(
707+
longText,
708+
maxLines = 1,
709+
overflow = TextOverflow.Ellipsis,
710+
textAlign = TextAlign.End,
711+
fontSize = 16.sp,
712+
modifier = Modifier.weight(1f).background(androidx.compose.ui.graphics.Color.LightGray),
713+
)
714+
}
715+
716+
// Short text (for comparison)
717+
Text(
718+
"Short text",
719+
fontSize = 16.sp,
720+
modifier = Modifier.background(androidx.compose.ui.graphics.Color.LightGray),
721+
)
722+
}
723+
}
724+
}
725+
}
726+
487727
private class MaskingActivity : Activity() {
488728

489729
override fun onCreate(savedInstanceState: Bundle?) {

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_all.png‎

-65 Bytes
Loading

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_ellipsized_compose_masked.png‎

2.85 KB
Loading

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_ellipsized_compose_unmasked.png‎

19.9 KB
Loading

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_ellipsized_view_masked.png‎

2.28 KB
Loading

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_ellipsized_view_unmasked.png‎

16.4 KB
Loading

‎sentry-android-core/src/test/resources/snapshots/ScreenshotEventProcessorTest/screenshot_mask_text.png‎

-62 Bytes
Loading

‎sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt‎

Lines changed: 29 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,39 @@ import androidx.compose.ui.node.LayoutNode
1313
import androidx.compose.ui.text.TextLayoutResult
1414
import kotlin.math.roundToInt
1515

16-
internal class ComposeTextLayout(
17-
internal val layout: TextLayoutResult,
18-
private val hasFillModifier: Boolean,
19-
) : TextLayout {
16+
internal class ComposeTextLayout(internal val layout: TextLayoutResult) : TextLayout {
2017
override val lineCount: Int
2118
get() = layout.lineCount
2219

2320
override val dominantTextColor: Int?
2421
get() = null
2522

26-
override fun getPrimaryHorizontal(line: Int, offset: Int): Float {
27-
val horizontalPos = layout.getHorizontalPosition(offset, usePrimaryDirection = true)
28-
// when there's no `fill` modifier on a Text composable, compose still thinks that there's
29-
// one and wrongly calculates horizontal position relative to node's start, not text's start
30-
// for some reason. This is only the case for single-line text (multiline works fien).
31-
// So we subtract line's left to get the correct position
32-
return if (!hasFillModifier && lineCount == 1) {
33-
horizontalPos - layout.getLineLeft(line)
34-
} else {
35-
horizontalPos
23+
/**
24+
* The paragraph may be laid out with a wider width (constraint maxWidth) than the actual node
25+
* (layout result size). When that happens, getLineLeft/getLineRight return positions in the
26+
* paragraph coordinate system, which don't match the node's bounds. In that case, text alignment
27+
* has no visible effect, so we fall back to using line width starting from x=0.
28+
*/
29+
private val paragraphWidthExceedsNode: Boolean
30+
get() = layout.multiParagraph.width > layout.size.width
31+
32+
override fun getLineLeft(line: Int): Float {
33+
if (paragraphWidthExceedsNode) {
34+
return 0f
3635
}
36+
return layout.getLineLeft(line)
3737
}
3838

39-
override fun getEllipsisCount(line: Int): Int = if (layout.isLineEllipsized(line)) 1 else 0
40-
41-
override fun getLineVisibleEnd(line: Int): Int = layout.getLineEnd(line, visibleEnd = true)
39+
override fun getLineRight(line: Int): Float {
40+
if (paragraphWidthExceedsNode) {
41+
return layout.multiParagraph.getLineWidth(line)
42+
}
43+
return layout.getLineRight(line)
44+
}
4245

4346
override fun getLineTop(line: Int): Int = layout.getLineTop(line).roundToInt()
4447

4548
override fun getLineBottom(line: Int): Int = layout.getLineBottom(line).roundToInt()
46-
47-
override fun getLineStart(line: Int): Int = layout.getLineStart(line)
4849
}
4950

5051
// TODO: probably most of the below we can do via bytecode instrumentation and speed up at runtime
@@ -92,46 +93,31 @@ internal fun Painter.isMaskable(): Boolean {
9293
!className.contains("Brush")
9394
}
9495

95-
internal data class TextAttributes(val color: Color?, val hasFillModifier: Boolean)
96-
9796
/**
9897
* This method is necessary to mask text in Compose.
9998
*
10099
* We heuristically look up for classes that have a [Text] modifier, usually they all have a `Text`
101100
* string in their name, e.g. TextStringSimpleElement or TextAnnotatedStringElement. We then get the
102101
* color from the modifier, to be able to mask it with the correct color.
103102
*
104-
* We also look up for classes that have a [Fill] modifier, usually they all have a `Fill` string in
105-
* their name, e.g. FillElement. This is necessary to workaround a Compose bug where single-line
106-
* text composable without a `fill` modifier still thinks that there's one and wrongly calculates
107-
* horizontal position.
108-
*
109103
* We also add special proguard rules to keep the `Text` class names and their `color` member.
110104
*/
111-
internal fun LayoutNode.findTextAttributes(): TextAttributes {
105+
internal fun LayoutNode.findTextColor(): Color? {
112106
val modifierInfos = getModifierInfo()
113-
var color: Color? = null
114-
var hasFillModifier = false
115107
for (index in modifierInfos.indices) {
116108
val modifier = modifierInfos[index].modifier
117109
val modifierClassName = modifier::class.java.name
118110
if (modifierClassName.contains("Text")) {
119-
color =
120-
try {
121-
(modifier::class
122-
.java
123-
.getDeclaredField("color")
124-
.apply { isAccessible = true }
125-
.get(modifier) as? ColorProducer)
126-
?.invoke()
127-
} catch (e: Throwable) {
128-
null
129-
}
130-
} else if (modifierClassName.contains("Fill")) {
131-
hasFillModifier = true
111+
return try {
112+
(modifier::class.java.getDeclaredField("color").apply { isAccessible = true }.get(modifier)
113+
as? ColorProducer)
114+
?.invoke()
115+
} catch (e: Throwable) {
116+
null
117+
}
132118
}
133119
}
134-
return TextAttributes(color, hasFillModifier)
120+
return null
135121
}
136122

137123
/**

0 commit comments

Comments
 (0)