Fix Firefox selection crash in Monaco: handle shadow DOM + null hit result in _doHitTestWithCaretPositionFromPoint#313960
Open
aesraethr wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens Monaco editor mouse hit-testing for Firefox in mouseTarget.ts, specifically the Gecko-specific caretPositionFromPoint path used during selection. It aligns the Firefox code path with the existing shadow-DOM-aware caret-range logic and prevents a null dereference when Firefox cannot resolve a caret position.
Changes:
- Pass the containing shadow root to
document.caretPositionFromPoint(...)when the editor is hosted inside shadow DOM. - Return
UnknownHitTestResultwhencaretPositionFromPoint(...)returnsnullinstead of dereferencingoffsetNode. - Add inline comments documenting the Firefox/shadow-DOM behavior motivating the defensive handling.
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
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.
Refs
bug+firefoxWhat
Two related defensive changes in
_doHitTestWithCaretPositionFromPoint(src/vs/editor/browser/controller/mouseTarget.ts):caretPositionFromPointso Firefox can resolve hits inside a shadow tree (parallels the_doHitTestWithCaretRangeFromPointsibling at line ~990, which already callsdom.getShadowRoot).hitResult.offsetNode— the API legitimately returnsnullfor points that don't fall on an addressable text node (e.g. dragging a multi-line selection past the end of the editor).Why
Firefox 131 changed
document.caretPositionFromPointso that, when the page uses shadow DOM, the third options argument must include{shadowRoots: [...]}to return the actual hit position. Without it, Firefox returns the shadow host (ornull). This crashes the editor's selection logic with:The error is fired through Monaco's
unexpectedErrorHandler, surfaces in the console, and pollutes any error-monitoring tool (Sentry, Datadog RUM) listening onwindow.onerror. The selection still works — it's a hit-test miss after the fact — but every multi-line drag triggers it.The bug is not just a shadow-DOM concern: contributors have reported the same
offsetNode is nullcrash on the Monaco playground in Firefox 145+ by simply selecting all text. That suggests Firefox'scaretPositionFromPointcan returnnulleven without shadow DOM in some drag scenarios, hence the explicitif (!hitResult) return new UnknownHitTestResult();guard.Reproduction
Fix attribution
The shadow-root passthrough was originally proposed in the issue thread by @nate-bow-db (Oct 2024). The community has been shipping it as a postinstall monkey-patch for over a year. This PR lands the same change in the source plus adds the explicit null-guard for the non-shadow-DOM crash path that surfaced more recently.
Risk / scope
mouseTarget.ts.dom.getShadowRoot(ctx.viewDomNode)returns non-null — so existing non-shadow-DOM consumers (the standard Monaco bundle) hit the same code path as before.new UnknownHitTestResult(), which is the same fallback the function already returns on its other failure path (line ~1080 today). No new return shape.dom.getShadowRootis already imported and used in the sibling_doHitTestWithCaretRangeFromPointmethod, so no new import is needed.