fix: capture dropdown clicks and attribute SVG button clicks - #4804
Open
marandaneto wants to merge 2 commits into
Open
fix: capture dropdown clicks and attribute SVG button clicks#4804marandaneto wants to merge 2 commits into
marandaneto wants to merge 2 commits into
Conversation
Contributor
|
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+203
to
+205
| if (controlIndex > 0) { | ||
| maskAllText = maskAllText || !shouldCaptureElement(target) || isSensitiveElement(target) | ||
| } |
Contributor
There was a problem hiding this comment.
When an SVG inside a button or link is clicked, this code decides whether to mask text using the SVG, then collects text from the enclosing control. If that control is sensitive because it is contenteditable or has a sensitive name or ID, its nested span text can still be included in $el_text.
How this was verified: Masking eligibility is evaluated against the SVG before the target is replaced with the enclosing control and text is collected from that control.
Knowledge Base Used: Browser event capture and autocapture
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/autocapture.ts
Line: 203-205
Comment:
**Sensitive control text leaks**
When an SVG inside a button or link is clicked, this code decides whether to mask text using the SVG, then collects text from the enclosing control. If that control is sensitive because it is contenteditable or has a sensitive name or ID, its nested span text can still be included in `$el_text`.
**How this was verified:** Masking eligibility is evaluated against the SVG before the target is replaced with the enclosing control and text is collected from that control.
**Knowledge Base Used:** [Browser event capture and autocapture](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-js/-/docs/browser-event-capture.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+256
to
+259
| if (controlIndex > 0) { | ||
| target = targetElementList[controlIndex] | ||
| elementsJson.splice(0, controlIndex) | ||
| } |
Contributor
There was a problem hiding this comment.
SVG normalization changes only the local target used to build $elements_chain. _captureEvent still calculates $element_selectors from the original SVG, so a watched selector matching the enclosing button or link is missing. Selector-based survey actions therefore fail to match these icon clicks even though the event is attributed to the control.
Knowledge Base Used: Browser event capture and autocapture
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/autocapture.ts
Line: 256-259
Comment:
**Control selectors stay unmatched**
SVG normalization changes only the local target used to build `$elements_chain`. `_captureEvent` still calculates `$element_selectors` from the original SVG, so a watched selector matching the enclosing button or link is missing. Selector-based survey actions therefore fail to match these icon clicks even though the event is attributed to the control.
**Knowledge Base Used:** [Browser event capture and autocapture](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-js/-/docs/browser-event-capture.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Contributor
|
Size Change: +26.4 kB (+0.13%) Total Size: 20.5 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
posthog-js Compliance ReportDate: 2026-09-05 22:02:39 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
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
1 participant
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.
Problem
Dropdown triggers can lose autocapture when opening a modal menu changes the eventual click target to the page root. Clicking a nested SVG icon also attributes the event to the SVG path instead of its button or link.
Related to #1526 and #991. The dropdown failure reproduces with Radix 2.1.4 and React 18.3.1 using the default
modal=true. The issue's explicitmodal=falseexample did not reproduce.The CI build also found five new private property names missing from the generated mangled-name list.
Changes
packages/browser/terser-mangled-names.jsonwith the CI build command to include the five pointer-recovery property names. No source, dependency, or configuration changes are included in this CI follow-up.Validation
dompurifyanddotenvstub definitions. No dependency or configuration workaround is included.WRITE_MANGLED_PROPERTIES=1 pnpm exec turbo --filter=posthog-js buildpassed twice. The second run rebuilt the browser package and produced an identical list. Cross-bundle property consistency and all 42 source-map checks passed. The three focused autocapture suites passed all 430 tests, and formatting and commit hooks passed.beac9aac9c3407af1386d565da022e99a7ef79a4againstf8013ed497fdf37765358df23152b328c339e586, with no findings. New CI results are pending.Recovery intentionally excludes legacy clicks without pointer identity, detached origins, and clicks delayed beyond pointer-state expiry. Radix Enter/Space interactions that prevent native clicks continue to produce no autocapture. Native button keyboard clicks remain captured once.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe existing patch changeset is included without adding a duplicate.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi implemented and validated the scoped fix, with independent source review and the isolated Pi autoreview helper before publication. Git, pnpm, Vitest, Playwright, and GitHub CLI were used. Session reference:
510ebebc-a750-45a5-88ce-92821ccd3256(local session, no public link).The fix targets the reproduced default-modal failure rather than claiming that the non-modal issue snippet failed. It preserves click-only capture instead of synthesizing events on pointerdown. Human review is required. This branch and the separate remote-config autocapture fix both touch
autocapture.ts; rerun their combined suites when integrating them.