Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-3868: Don't traverse non-visible ViewGroups for searching user interaction targets #1969

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ internal class GesturesListener(
stack: LinkedList<View>,
coordinatesContainer: IntArray
) {
if (!view.isVisible) return

for (i in 0 until view.childCount) {
val child = view.getChildAt(i)
if (hitTest(child, x, y, coordinatesContainer)) {
Expand All @@ -290,11 +292,11 @@ internal class GesturesListener(
}

private fun isValidTapTarget(view: View): Boolean {
return view.isClickable && view.visibility == View.VISIBLE
return view.isClickable && view.isVisible
}

private fun isValidScrollableTarget(view: View): Boolean {
return view.visibility == View.VISIBLE && isScrollableView(view)
return view.isVisible && isScrollableView(view)
}

@Suppress("UnsafeThirdPartyFunctionCall") // NPE cannot happen here
Expand Down Expand Up @@ -345,6 +347,9 @@ internal class GesturesListener(
return view::class.java.name.startsWith("androidx.compose.ui.platform.ComposeView")
}

private val View.isVisible: Boolean
get() = visibility == View.VISIBLE

// endregion

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,51 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
verifyNoInteractions(rumMonitor.mockInstance)
}

@Test
fun `will do nothing if target is in non-visible ViewGroup `(forge: Forge) {
val startDownEvent: MotionEvent = forge.getForgery()
val listSize = forge.anInt(1, 20)
val intermediaryEvents =
forge.aList(size = listSize) { forge.getForgery(MotionEvent::class.java) }
val distancesX = forge.aList(listSize) { forge.aFloat() }
val distancesY = forge.aList(listSize) { forge.aFloat() }
val targetId = forge.anInt()
// ensure the last event is within the bounds of the target
val endUpEvent = startDownEvent
val targetView = mockView<View>(
id = targetId,
forEvent = startDownEvent,
hitTest = true,
forge = forge
)
mockDecorView = mockDecorView<ViewGroup>(
id = forge.anInt(),
forEvent = startDownEvent,
hitTest = true,
forge = forge
) {
whenever(it.visibility).thenReturn(forge.anElementFrom(View.INVISIBLE, View.GONE))
whenever(it.childCount).thenReturn(1)
whenever(it.getChildAt(0)).thenReturn(targetView)
}
testedListener = GesturesListener(
rumMonitor.mockSdkCore,
WeakReference(mockWindow),
contextRef = WeakReference(mockAppContext),
internalLogger = mockInternalLogger
)

// When
testedListener.onDown(startDownEvent)
intermediaryEvents.forEachIndexed { index, event ->
testedListener.onScroll(startDownEvent, event, distancesX[index], distancesY[index])
}
testedListener.onUp(endUpEvent)

// Then
verifyNoInteractions(rumMonitor.mockInstance)
}

@Test
fun `will do nothing and not log warning if target is in Jetpack Compose view `(forge: Forge) {
val startDownEvent: MotionEvent = forge.getForgery()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,43 @@ internal class GesturesListenerTapTest : AbstractGesturesListenerTest() {
verifyMonitorCalledWithUserAction(validTarget, "", expectedResourceName)
}

@Test
fun `onTap does nothing if not visible ViewGroup contains visible views`(forge: Forge) {
// Given
val mockEvent: MotionEvent = forge.getForgery()
val validTarget: View = mockView(
id = forge.anInt(),
forEvent = mockEvent,
hitTest = true,
forge = forge,
clickable = true
)
mockDecorView = mockDecorView<ViewGroup>(
id = forge.anInt(),
forEvent = mockEvent,
hitTest = true,
forge = forge
) {
whenever(it.visibility).thenReturn(forge.anElementFrom(View.INVISIBLE, View.GONE))
whenever(it.childCount).thenReturn(1)
whenever(it.getChildAt(0)).thenReturn(validTarget)
}
val expectedResourceName = forge.anAlphabeticalString()
mockResourcesForTarget(validTarget, expectedResourceName)
testedListener = GesturesListener(
rumMonitor.mockSdkCore,
WeakReference(mockWindow),
contextRef = WeakReference(mockAppContext),
internalLogger = mockInternalLogger
)

// When
testedListener.onSingleTapUp(mockEvent)

// Then
verifyNoInteractions(rumMonitor.mockInstance)
}

@Test
fun `onTap does nothing if no children present and decor view not clickable`(
forge: Forge
Expand Down