Skip to content

fix(selection-toolbar): ignore overlay text selections#1224

Merged
mengxi-ream merged 2 commits intomainfrom
fix/selection-toolbar-ignore-overlay-selection
Mar 24, 2026
Merged

fix(selection-toolbar): ignore overlay text selections#1224
mengxi-ream merged 2 commits intomainfrom
fix/selection-toolbar-ignore-overlay-selection

Conversation

@ananaBMaster
Copy link
Copy Markdown
Collaborator

Type of Changes

  • ✨ New feature (feat)
  • 🐛 Bug fix (fix)
  • 📝 Documentation change (docs)
  • 💄 UI/style change (style)
  • ♻️ Code refactoring (refactor)
  • ⚡ Performance improvement (perf)
  • ✅ Test related (test)
  • 🔧 Build or dependencies update (build)
  • 🔄 CI/CD related (ci)
  • 🌐 Internationalization (i18n)
  • 🧠 AI model related (ai)
  • 🔄 Revert a previous commit (revert)
  • 📦 Other changes that do not modify src or test files (chore)

Description

Prevent selection toolbar state from being replaced when users select text inside the extension's own overlay UI.

  • mark selection toolbar surfaces, popovers, and toaster output as selection-overlay roots
  • ignore mousedown, mouseup, and selectionchange events that originate from overlay selections
  • preserve the existing page selection session so rerunning toolbar actions still uses the original page paragraphs
  • add a regression test covering selection inside the translation popover followed by a rerun

Related Issue

Closes #

How Has This Been Tested?

  • Added unit tests

  • Verified through manual testing

  • SKIP_FREE_API=true pnpm vitest run src/entrypoints/selection.content/selection-toolbar/__tests__/request-rerun.test.tsx src/entrypoints/selection.content/selection-toolbar/__tests__/selection-toolbar.test.tsx

  • SKIP_FREE_API=true pnpm type-check

Screenshots

N/A

Checklist

  • I have tested these changes locally
  • I have updated the documentation accordingly if necessary
  • My code follows the code style of this project
  • My changes do not break existing functionality
  • If my code was generated by AI, I have proofread and improved it as necessary.

Additional Information

This intentionally fixes the event/session boundary at the selection overlay layer without changing paragraph extraction logic.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 24, 2026

🦋 Changeset detected

Latest commit: e0049ea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@read-frog/extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. app: browser extension Related to browser extension labels Mar 24, 2026
@dosubot
Copy link
Copy Markdown

dosubot bot commented Mar 24, 2026

Documentation Updates

1 document(s) were updated by changes in this PR:

Translation Toggle Logic and Content Detection
View Changes
@@ -131,6 +131,13 @@
 As of v1.31.3 (PR #1185), retargeted interactive clicks are properly filtered. The implementation walks through the event's composed path to identify the true interactive element that was clicked, regardless of shadow DOM boundaries or event retargeting mechanisms. This ensures that clicks originating from buttons or other interactive elements inside shadow roots are correctly recognized, even when the event's `target` property is retargeted to the shadow host element.
 
 The implementation checks whether the interactive element is contained within the current selection using `selection.containsNode()`. If the interactive element is outside the selection, the toolbar is hidden; if it's inside the selection, the toolbar remains visible. This ensures intuitive behavior when users interact with translation controls within the toolbar itself while preventing unexpected toolbar reappearance when clicking unrelated page controls.
+
+**Overlay Selection Filtering:**
+The selection toolbar filters out text selections made within its own overlay UI elements to prevent interference with the main page selection. Overlay elements (popovers, toaster, toolbar surfaces) are marked with the `data-rf-selection-overlay-root` attribute, and the toolbar detects and ignores selection events originating from these marked elements.
+
+When users interact with overlay UI (e.g., selecting text in a translation result, clicking buttons in the toolbar), the system preserves the original page selection session rather than replacing it. This ensures that rerunning toolbar actions (such as regenerating a translation) continues to use the original page paragraphs and context, preventing unwanted toolbar state changes and maintaining a consistent workflow.
+
+The filtering logic inspects `mousedown`, `mouseup`, and `selectionchange` events to determine if they originate from overlay elements. Selection boundary nodes (anchor nodes, focus nodes, and range containers) are checked against the overlay container and any elements with the overlay root attribute. If the selection is contained within an overlay, the main selection state is preserved and the toolbar does not update or dismiss inappropriately.
 
 **Translation Flow and Request Handling:**
 The selection toolbar translation flow is designed for reliability and responsiveness. The system includes hardened stale-request cancellation handling to manage rapid selection changes. When a user makes a selection and then quickly changes it, pending translation requests for the previous selection are cancelled, ensuring that only the most current selection is translated. This prevents race conditions and ensures that outdated translations do not overwrite current content, providing a smooth and predictable translation experience.
@@ -187,6 +194,14 @@
 - The alert component uses the `role="alert"` attribute for accessibility and proper testing
 - Custom action errors follow the same inline alert pattern for consistency across all selection toolbar features
 
+**Overlay Selection Filtering Tests:**
+Integration tests in `src/entrypoints/selection.content/selection-toolbar/__tests__/request-rerun.test.tsx` and `src/entrypoints/selection.content/selection-toolbar/__tests__/selection-toolbar.test.tsx` verify overlay selection filtering behavior:
+- When users select text inside the translation popover and then rerun the translation, the original page selection and context are preserved
+- Selection boundaries (anchor node, focus node, range containers) inside overlay root elements are correctly detected and filtered
+- Text selections within elements marked with `data-rf-selection-overlay-root` do not trigger toolbar state changes
+- The toolbar does not show or hide inappropriately when selections are made inside overlay UI elements
+- Rerunning toolbar actions after overlay interactions uses the original page paragraphs rather than overlay content
+
 **Float Layout Tests:**
 Integration tests verify the float layout detection and handling:
 - In bilingual mode, block translations are marked with `data-read-frog-float-wrap="true"` when the translated content would drop below an active floated sibling

How did I do? Any feedback?  Join Discord

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/entrypoints/selection.content/selection-toolbar/index.tsx">

<violation number="1" location="src/entrypoints/selection.content/selection-toolbar/index.tsx:89">
P2: Overlay-root detection misses text-node selections because the attribute check only runs for `Element` nodes.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/entrypoints/selection.content/selection-toolbar/index.tsx Outdated
@github-actions github-actions bot added the fix label Mar 24, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82a1c22369

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +89 to +90
if (node instanceof Element && node.closest(SELECTION_OVERLAY_ROOT_SELECTOR)) {
return true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle text-node boundaries when matching overlay roots

isNodeInsideSelectionOverlay only runs the overlay-root attribute check when the boundary node is an Element, but browser selections usually report Text nodes for anchorNode/focusNode and range boundaries. In cases where overlay UI is identified via data-rf-selection-overlay-root rather than overlayContainer.contains(...)/shared shadow root (for example, portal content rendered outside the toolbar container), this returns false and the toolbar can still treat overlay selections as page selections. Resolve this by walking from non-Element nodes to their parent element before calling closest.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/entrypoints/selection.content/selection-toolbar/__tests__/selection-toolbar.test.tsx">

<violation number="1" location="src/entrypoints/selection.content/selection-toolbar/__tests__/selection-toolbar.test.tsx:406">
P2: This test leaks DOM state by appending an overlay root to `document.body` without removing it, which can make later tests order-dependent.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@mengxi-ream mengxi-ream merged commit 64931e3 into main Mar 24, 2026
6 checks passed
@mengxi-ream mengxi-ream deleted the fix/selection-toolbar-ignore-overlay-selection branch March 24, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: browser extension Related to browser extension fix size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants