Repository navigation
fix(plugin-autocapture-browser): do not capture element text for 'contenteditable' - #2066
daniel-graham-amplitude wants to merge 1 commit into
Conversation
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
542b379 to
cb954f7
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb954f7684
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| getText = (element: Element): string => { | ||
| // Editable regions hold user-entered text. Omit it so Element Text is dropped. | ||
| if (isContentEditableElement(element)) { |
There was a problem hiding this comment.
When the tracked element is a descendant of a contenteditable region, this check returns false because the descendant usually has no attribute of its own, even though editability is inherited. For example, a click on a nested button, a custom-allowlisted span, or an action-click descendant inside contenteditable="plaintext-only" causes getText to read and transmit that descendant's user-entered text. Determine effective editability (respecting the nearest contenteditable="false" boundary) rather than checking only the element itself.
Useful? React with 👍 / 👎.
cb954f7 to
b3e27af
Compare
|
@codex review |
|
bugbot run |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3e27af68b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (current instanceof HTMLElement) { | ||
| const value = current.getAttribute('contenteditable'); | ||
| if (value !== null) { | ||
| return value.toLowerCase() !== 'false'; |
There was a problem hiding this comment.
When markup uses an invalid enumerated value such as contenteditable="inherit", the HTML standard defines it as inherited, but this branch treats every value except false as editable. On a standalone element or below contenteditable="false", getText now returns empty and hierarchy attribute capture is suppressed even though the browser considers the element noneditable, silently dropping ordinary analytics data. Recognize only the empty, true, and plaintext-only states as editable and continue walking for invalid values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b3e27af. Configure here.
| element instanceof HTMLElement ? element.getAttribute('contenteditable')?.toLowerCase() === 'true' : false; | ||
|
|
||
| return !SENSITIVE_TAGS.includes(tag) && !isContentEditable; | ||
| return !SENSITIVE_TAGS.includes(tag) && !isContentEditableElement(element); |
There was a problem hiding this comment.
Editable descendants lose hierarchy attributes
Medium Severity
isNonSensitiveElement now calls isContentEditableElement, so any node inside an editing host is treated as sensitive. Hierarchy collection then drops attributes such as aria-label and data-amp-track-* on nested buttons, links, and chips. The change is scoped to omitting Element Text, so those identifying properties disappear on clicks inside editors.
Reviewed by Cursor Bugbot for commit b3e27af. Configure here.
This branch has not been deployed
2 participants
Summary
PROBLEM:
When an element has the contenteditable attribute it means that the element is "editable by the user". This means that the inner text of a "contenteditable element" is semantically equivalent to the "value" attribute on an input element; and it should be excluded from autocapture.
SOLUTION:
Do not capture
Element Texton elements that have the "contenteditable" attribute.Checklist
Note
Low Risk
Privacy-focused autocapture change that reduces PII leakage from rich text editors; behavior may differ for apps that relied on Element Text inside contenteditable regions.
Overview
Stops autocapture from sending Element Text for user-editable DOM, treating
contenteditablelike sensitive form fields.Adds
isContentEditableElement, which walks ancestors for the nearestcontenteditableattribute (any value except"false", includingplaintext-onlyand empty).getTextreturns empty for editable elements so[Amplitude] Element Textis dropped from event properties; descendants inside an editing host are omitted unless a closercontenteditable="false"island restores static text.isNonSensitiveElementnow uses the same helper instead of only checkingcontenteditable="true"on the element itself.Tests and the autocapture test-server page document inherited editability and nested
contenteditable="false"boundaries.Reviewed by Cursor Bugbot for commit b3e27af. Bugbot is set up for automated code reviews on this repo. Configure here.