From ed2f75fe439a8dc63d0beaba7895dd001492072d Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Thu, 29 Oct 2026 16:29:24 -0400 Subject: [PATCH 1/2] refactor(replay): share one mask classifier between the mask walk and the discard check --- .changeset/shared-mask-rule.md | 5 + .../replay/PostHogReplayIntegration.kt | 91 ++++++++----------- .../replay/PostHogReplayIntegrationTest.kt | 45 +++++++++ 3 files changed, 86 insertions(+), 55 deletions(-) create mode 100644 .changeset/shared-mask-rule.md diff --git a/.changeset/shared-mask-rule.md b/.changeset/shared-mask-rule.md new file mode 100644 index 000000000..9de2fef3e --- /dev/null +++ b/.changeset/shared-mask-rule.md @@ -0,0 +1,5 @@ +--- +"posthog-android": patch +--- + +Refactor session replay masking so the mask walk and the transient-view discard check share one classifier, without changing SDK behavior. diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 6fde07b7a..5e14bae5e 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -1134,9 +1134,8 @@ public class PostHogReplayIntegration( var walkChildren = false - // Keep in sync with hasMaskableContent. - when { - view.isComposeView() -> { + when (view.maskRule()) { + MaskRule.COMPOSE -> { findMaskableComposeWidgets(view, walk) if (walk.shouldStop) { return @@ -1144,44 +1143,10 @@ public class PostHogReplayIntegration( // Also walk View children for interop scenarios (AndroidView, FragmentContainerView, etc.) walkChildren = true } - - view.isUnmasked() -> { - // ph-no-mask has precedence, skip masking - } - - view.isNoCapture() -> { - view.addGlobalVisibleRect(walk) - } - - view is TextView -> { - // Only emptiness matters here; toString() would copy the text on every walk. - val hasContent = view.text?.isNotEmpty() == true || view.hint?.isNotEmpty() == true - if (hasContent && view.shouldMaskTextView()) { - view.addTextAreaGlobalVisibleRect(walk) - } - } - - view is Spinner -> { - if (view.shouldMaskSpinner()) { - view.addGlobalVisibleRect(walk) - } - } - - view is ImageView -> { - if (view.shouldMaskImage()) { - view.addGlobalVisibleRect(walk) - } - } - - view is WebView -> { - if (view.isAnyInputSensitive()) { - view.addGlobalVisibleRect(walk) - } - } - - view is ViewGroup && view.childCount > 0 -> { - walkChildren = true - } + MaskRule.MASK_VIEW -> view.addGlobalVisibleRect(walk) + MaskRule.MASK_TEXT -> (view as TextView).addTextAreaGlobalVisibleRect(walk) + MaskRule.CHILDREN -> walkChildren = true + MaskRule.NONE -> Unit } if (walkChildren && view is ViewGroup && view.childCount > 0) { @@ -1208,27 +1173,43 @@ public class PostHogReplayIntegration( } } + internal enum class MaskRule { COMPOSE, MASK_VIEW, MASK_TEXT, CHILDREN, NONE } + + // The one place that decides how a view is masked, shared by the mask walk and + // hasMaskableContent so the two can't disagree. + internal fun View.maskRule(): MaskRule = + when { + isComposeView() -> MaskRule.COMPOSE + // ph-no-mask has precedence, skip masking + isUnmasked() -> MaskRule.NONE + isNoCapture() -> MaskRule.MASK_VIEW + // Only emptiness matters here; toString() would copy the text on every walk. + this is TextView -> + if ((text?.isNotEmpty() == true || hint?.isNotEmpty() == true) && shouldMaskTextView()) { + MaskRule.MASK_TEXT + } else { + MaskRule.NONE + } + this is Spinner -> if (shouldMaskSpinner()) MaskRule.MASK_VIEW else MaskRule.NONE + this is ImageView -> if (shouldMaskImage()) MaskRule.MASK_VIEW else MaskRule.NONE + this is WebView -> if (isAnyInputSensitive()) MaskRule.MASK_VIEW else MaskRule.NONE + this is ViewGroup && childCount > 0 -> MaskRule.CHILDREN + else -> MaskRule.NONE + } + // Whether this subtree holds anything the walk would mask, judged without geometry, so a // legacy walk can tell an unplaceable sensitive view from a harmless animation. - // Keep in sync with findMaskableWidgets. - private fun View.hasMaskableContent(walk: MaskWalk): Boolean { + internal fun View.hasMaskableContent(walk: MaskWalk): Boolean { if (!walk.visitedViews.add(System.identityHashCode(this))) { return false } - return when { - isComposeView() -> true - isUnmasked() -> false - isNoCapture() -> true - this is TextView -> - (text?.isNotEmpty() == true || hint?.isNotEmpty() == true) && shouldMaskTextView() - this is Spinner -> shouldMaskSpinner() - this is ImageView -> shouldMaskImage() - this is WebView -> isAnyInputSensitive() - this is ViewGroup -> - (0 until childCount).any { i -> + return when (maskRule()) { + MaskRule.COMPOSE, MaskRule.MASK_VIEW, MaskRule.MASK_TEXT -> true + MaskRule.CHILDREN -> + (0 until (this as ViewGroup).childCount).any { i -> getChildAt(i)?.let { it.visibility == View.VISIBLE && it.hasMaskableContent(walk) } == true } - else -> false + MaskRule.NONE -> false } } diff --git a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt index 769e60ed6..811826954 100644 --- a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt @@ -15,10 +15,12 @@ import android.util.Base64 import android.view.MotionEvent import android.view.PixelCopy import android.view.View +import android.view.ViewGroup import android.view.Window import android.view.WindowManager import android.view.animation.Animation import android.widget.FrameLayout +import android.widget.ImageView import android.widget.TextView import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -4328,6 +4330,49 @@ internal class PostHogReplayIntegrationTest { } } + @Test + fun `the discard check treats as maskable exactly what the mask walk masks`() { + val (fx, _) = screenshotFixture(enableMaskAlignmentVerification = false) + try { + val activity = Robolectric.buildActivity(Activity::class.java).setup().get() + val root = FrameLayout(activity) + activity.setContentView(root) + val views = + listOf( + View(activity), + View(activity).apply { tag = "ph-no-capture" }, + TextView(activity), + TextView(activity).apply { text = "secret" }, + TextView(activity).apply { + text = "secret" + tag = "ph-no-mask" + }, + ImageView(activity).apply { setImageBitmap(Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888)) }, + FrameLayout(activity).apply { addView(TextView(activity).apply { text = "secret" }) }, + FrameLayout(activity).apply { addView(View(activity)) }, + ) + views.forEach { root.addView(it) } + shadowOf(Looper.getMainLooper()).idle() + makeWindowVisible(activity.window.decorView) + root.layout(0, 0, 100, 100) + views.forEach { view -> + view.layout(0, 0, 100, 20) + (view as? ViewGroup)?.getChildAt(0)?.layout(0, 0, 100, 20) + } + + val maskedByWalk = + views.map { view -> + PostHogReplayIntegration.MaskWalk(failClosed = false).also { fx.sut.findMaskableWidgets(view, it) }.rects.isNotEmpty() + } + val maskableForDiscard = views.map { with(fx.sut) { it.hasMaskableContent(PostHogReplayIntegration.MaskWalk()) } } + + assertEquals(listOf(false, true, false, true, false, true, true, false), maskedByWalk) + assertEquals(maskedByWalk, maskableForDiscard) + } finally { + fx.sut.uninstall() + } + } + @Test fun `legacy mask walk is not poisoned by an animating view with nothing to mask`() { val (fx, _) = screenshotFixture(enableMaskAlignmentVerification = false) From 1238d7408771beac8ce7a4e0b990d045ab22506e Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Thu, 29 Oct 2026 17:00:09 -0400 Subject: [PATCH 2/2] test(replay): cover Compose and hidden children in the mask classifier parity test Drops the refactor changeset: an internal refactor has no user-facing changelog entry. --- .changeset/shared-mask-rule.md | 5 ----- .../replay/PostHogReplayIntegrationTest.kt | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 8 deletions(-) delete mode 100644 .changeset/shared-mask-rule.md diff --git a/.changeset/shared-mask-rule.md b/.changeset/shared-mask-rule.md deleted file mode 100644 index 9de2fef3e..000000000 --- a/.changeset/shared-mask-rule.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"posthog-android": patch ---- - -Refactor session replay masking so the mask walk and the transient-view discard check share one classifier, without changing SDK behavior. diff --git a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt index 204c84f83..a653ea2e7 100644 --- a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt @@ -4333,7 +4333,7 @@ internal class PostHogReplayIntegrationTest { } @Test - fun `the discard check treats as maskable exactly what the mask walk masks`() { + fun `the discard check treats everything the mask walk masks as maskable`() { val (fx, _) = screenshotFixture(enableMaskAlignmentVerification = false) try { val activity = Robolectric.buildActivity(Activity::class.java).setup().get() @@ -4352,6 +4352,15 @@ internal class PostHogReplayIntegrationTest { ImageView(activity).apply { setImageBitmap(Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888)) }, FrameLayout(activity).apply { addView(TextView(activity).apply { text = "secret" }) }, FrameLayout(activity).apply { addView(View(activity)) }, + FrameLayout(activity).apply { + addView( + TextView(activity).apply { + text = "secret" + visibility = View.INVISIBLE + }, + ) + }, + FakeAndroidComposeView(activity), ) views.forEach { root.addView(it) } shadowOf(Looper.getMainLooper()).idle() @@ -4368,8 +4377,9 @@ internal class PostHogReplayIntegrationTest { } val maskableForDiscard = views.map { with(fx.sut) { it.hasMaskableContent(PostHogReplayIntegration.MaskWalk()) } } - assertEquals(listOf(false, true, false, true, false, true, true, false), maskedByWalk) - assertEquals(maskedByWalk, maskableForDiscard) + assertEquals(listOf(false, true, false, true, false, true, true, false, false, false), maskedByWalk) + // Compose content can't be judged without geometry, so the discard check counts it as maskable. + assertEquals(listOf(false, true, false, true, false, true, true, false, false, true), maskableForDiscard) } finally { fx.sut.uninstall() }