Fix select, nested scroll, and Shadow DOM interactions - #17
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis change improves Firefox computer-use support for strict-CSP pages, open Shadow DOM pointer targeting, nested scrolling, and keyboard selection. Executed checks confirmed that pointer actions reach the innermost accessible open-shadow element, nested scroll gestures use an available container before the page, disabled select options are skipped, and a page-defined helper is replaced. Two input regressions remain in Confidence Score: 5/5Do not merge until the wheel-cancellation and multi-select selection regressions in Every investigated behavior was exercised through the Firefox compatibility bridge with captured before/after evidence. Both remaining failures were reproduced directly, and the shadow targeting, nested scrolling, disabled-option, and strict-CSP helper behaviors were verified by execution. Files Needing Attention:
What T-Rex did
|
| target.dispatchEvent(new WheelEvent("wheel", { ...common, deltaX, deltaY, deltaMode: WheelEvent.DOM_DELTA_PIXEL })); | ||
| scrollAtPoint(x, y, deltaX, deltaY); |
There was a problem hiding this comment.
Canceled wheel events still scroll
The bridge dispatches a cancelable wheel event but ignores whether a page listener canceled it before unconditionally calling scrollAtPoint. A listener that calls preventDefault() observes defaultPrevented: true, yet the nested container still scrolls by the requested delta. Honor dispatchEvent()'s boolean return, or event.defaultPrevented, before applying the synthetic scroll.
| } else if (target instanceof HTMLSelectElement && (key === "ArrowDown" || key === "ArrowUp")) { | ||
| const direction = key === "ArrowDown" ? 1 : -1; | ||
| let nextIndex = target.selectedIndex; | ||
| do { | ||
| nextIndex += direction; | ||
| } while (nextIndex >= 0 && nextIndex < target.options.length && target.options[nextIndex].disabled); | ||
| if (nextIndex >= 0 && nextIndex < target.options.length && nextIndex !== target.selectedIndex) { | ||
| target.selectedIndex = nextIndex; | ||
| target.dispatchEvent(new Event("input", { bubbles: true, composed: true })); | ||
| target.dispatchEvent(new Event("change", { bubbles: true })); |
There was a problem hiding this comment.
Arrow navigation destroys multi-select values
This single-select navigation path also runs for <select multiple>. Assigning selectedIndex clears the existing selection set: an exercised control initially selecting Alpha and Beta retained only Beta after one ArrowDown, then emitted input and change. Exclude target.multiple controls unless modifier-aware multi-select behavior is implemented.
Summary
Verification
npm testpasses on the clean v1.4.10 branchnode tests/test-firefox-compat.mjspassesgit diff --checkpassesFirefox Bridge Live|Gamma|true,frame-bridge-live,shadow-clicked, andInner scroll target reachedScope
This PR makes no general browser-parity claim. It covers the tested enabled single-select, nested vertical scrolling, open Shadow DOM hit-testing, nested-frame, and direct strict-CSP DOM-domain paths. Closed Shadow DOM, broader selector semantics, and first-attempt latency remain outside this change.