RUM-16187: Fix tap target mis-attribution for views clipped by scrolling ancestors#3473
Merged
Conversation
…ing ancestors The SDK's gesture target resolution used View.getLocationInWindow + width x height to compute a view's window-coordinate rectangle for hit-testing. This returns the view's raw layout bounds and does not account for ancestor clipping. As a result, a clickable view scrolled past its parent's visible region (e.g. a Button inside a NestedScrollView whose layout position falls behind a sibling BottomNavigationView) was still considered a valid tap target. Because the SDK's BFS traversal visits the deepest matching view last and overwrites the target on every later match, taps on the BottomNavigationView were attributed to the (visually hidden) underlying button instead of the nav-bar item. Switch hitTest in AndroidActionTrackingStrategy to use View.getGlobalVisibleRect. This returns the view's bounds intersected with all ancestor clip rectangles, so views that are scrolled out of their parent's visible area no longer satisfy the hit-test. Both APIs return coordinates in the window coordinate space, so the comparison against MotionEvent coordinates is unchanged.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3473 +/- ##
===========================================
+ Coverage 72.21% 72.27% +0.06%
===========================================
Files 964 964
Lines 35554 35552 -2
Branches 5922 5924 +2
===========================================
+ Hits 25673 25694 +21
+ Misses 8263 8242 -21
+ Partials 1618 1616 -2
🚀 New features to boost your workflow:
|
hamorillo
marked this pull request as ready for review
May 27, 2026 06:32
0xnm
previously approved these changes
May 27, 2026
0xnm
left a comment
0xnm
left a comment
Member
There was a problem hiding this comment.
Nice change 👍 One concern is that it may change action-to-view attribution in customer data (in widgets, monitors, etc.), but I don't think it will be dramatic and anyway this change is for correctness.
ambushwork
previously approved these changes
May 27, 2026
jonathanmos
previously approved these changes
May 27, 2026
- Drop unnecessary Float-to-Int conversions in hitTest; Float compares with Int as-is in Kotlin - Add test covering the clipped-scrollable case for the scroll path (mirrors the existing tap test) - Remove the misleading comment about Rect.set() being a no-op stub - Remove the now-unused getLocationInWindow mock from the mockView helper - Remove the orphaned getLocationInWindow entry from detekt_custom_unsafe_calls.yml
hamorillo
dismissed stale reviews from jonathanmos, ambushwork, and 0xnm
via
May 27, 2026 09:13
6320f08
0xnm
approved these changes
May 27, 2026
jonathanmos
approved these changes
May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Replaces
View.getLocationInWindow + width × heightwithView.getGlobalVisibleRectinAndroidActionTrackingStrategy.hitTestso the gesture target resolver respects ancestor clipping.Before this change, a clickable view scrolled past its parent's visible region (for example, a
Buttoninside aNestedScrollViewwhose layout position falls behind a siblingBottomNavigationView) was still considered a valid tap target. BecausefindTargetperforms a BFS that overwritestargeton every later match, the visually hidden button was visited after the legitimateBottomNavigationItemViewand won the resolution, causing taps on theBottomNavigationViewto be attributed to the hidden underlying button.getGlobalVisibleRectreturns the view's bounds intersected with all ancestor clip rectangles, so views that are scrolled out of their parent's visible area no longer satisfy the hit-test. Both APIs return coordinates in the window coordinate space, so the comparison againstMotionEventcoordinates is unchanged.Motivation
RUM-16187 (escalated from RUMS-5868).
Customers report that taps on
BottomNavigationViewitems get mis-attributed to underlyingButtonviews inside theViewPager2page when those buttons sit at scroll positions that place their raw layout rect behind the nav bar. The behaviour was reproduced on stock SDK 3.9.1 and 3.10.0 in a controlled repro app mirroring the customer's layout (ConstraintLayoutwithViewPager2.bottom_toTopOf="@id/separator",separator.bottom_toTopOf="@id/bottom_nav",BottomNavigationView.bottom_toBottomOf="parent"). At the initial scroll position of theNestedScrollView, the last-but-one and last items have layout positions that fall in the window-coord range occupied by the nav bar (item_11 aty ∈ [2132, 2321]for a tap aty = 2232). The stock SDK's raw-bounds hit-test reported these as valid targets.Additional Notes
getLocationInWindowandgetGlobalVisibleRectreturn window-space coordinates (top-left of the decorView is origin) per the AOSP Javadocs. TheMotionEventcoordinates received viaWindow.Callback.dispatchTouchEventare in the same space, so no conversion is needed.The fix was verified end-to-end with
dd-sdk-androidpublished to Maven local against a repro app that mirrors the customer-reported layout. With SDK 3.10.0, three of three overlap-state Messages-tab taps were attributed toMaterialButton(item_11). With this patch applied, all three were correctly attributed toBottomNavigationItemView(nav_messages).Review checklist (to be filled by reviewers)