Repository navigation
Conversation
…WebKit crash) With `enableGutterUtility`, InteractionManager repositioned the gutter utility button on every pointermove by re-parenting it into the hovered line's number cell (`showUtilityOnLine` -> `numberElement.appendChild`). During a native text-selection drag, pointermove fires continuously as the selection auto-scrolls, so the utility node was moved between line cells under the in-progress selection. WebKit crashes its web-content process on that mid-drag DOM mutation (EXC_BREAKPOINT in EventHandler::handleMouseDraggedEvent, pierre#1184; Safari and WKWebView hosts such as Electrobun). Track native text-selection drags (a mouse button held while no pierre gutter/line-selection session owns the pointer) via `nativeTextDragActive` and make `placeUtility()` a no-op while it is set, so the utility node is not moved or removed during the drag. Placement resumes on the next hover move after the drag ends. The flag is armed in `handlePointerMove` and cleared on a fresh `handlePointerDown`. Adds a browser-agnostic regression test: during a native text-selection drag the gutter utility stays on its original line instead of following the pointer (and, on affected WebKit builds, no longer crashes).
|
@OLavrik is attempting to deploy a commit to the Pierre Computer Company Team on Vercel. A member of the Team first needs to authorize it. |
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: 576e6c5742
ℹ️ 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".
| this.nativeTextDragActive = | ||
| event.buttons !== 0 && this.pointerSession.mode === 'idle'; |
There was a problem hiding this comment.
If a native primary-button drag leaves the <pre> before any pointermove is delivered to it (for example, the selection begins near an edge and the first sampled move lands outside), this assignment never runs. handlePointerLeave() still clears hoveredLine and calls placeUtility(), so with the flag still false it removes or reparents the utility during the active native selection—the same WebKit mutation/crash this patch is intended to prevent. Arm the state from the held-button pointerleave (or earlier) before allowing placement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in c77568e. handlePointerLeave now arms nativeTextDragActive (via the shared isNativeTextDrag predicate) before it clears hoveredLine and calls placeUtility(), so a drag whose first sampled move lands outside the <pre> no longer removes/reparents the utility under the active native selection.
| this.nativeTextDragActive = | ||
| event.buttons !== 0 && this.pointerSession.mode === 'idle'; |
There was a problem hiding this comment.
When a drag ends over the line of its last held-button move, those moves have already updated hoveredLine while this guard kept the utility at its original row. The first no-button move only clears the flag here; handlePointerEvent() then sees sameLine and skips its placeUtility() calls, leaving the button on the old row until the pointer crosses another line or leaves the <pre>. Re-place when the state transitions from true to false so hover behavior actually resumes after the drag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c77568e. handlePointerMove now tracks the previous flag value and, on the true->false transition, calls placeUtility() after handlePointerEvent, so when a drag ends on the same line as its last held-button move the utility re-places on the current hovered row instead of staying on the pre-drag row. Regression test extended to assert the utility follows hover again after the drag.
Drag-selecting text in a code diff crashed WebKit's web-content process; on the Electrobun desktop (macOS WKWebView) the workspace reloaded into "A problem repeatedly occurred". Root cause is upstream (pierrecomputer/pierre#1184): with the gutter utility enabled, Pierre's InteractionManager re-parented the utility button into the hovered line's number cell on every pointermove, so a native selection drag mutated the DOM under the in-progress selection and tripped a WebKit assert (EXC_BREAKPOINT in WebCore::EventHandler::handleMouseDraggedEvent). It is not virtualization and not the diff's grid layout: freezing Pierre's row rendering does not prevent it. Carry the upstream fix (pierrecomputer/pierre#1185) as a bun patch until a Pierre release ships it, instead of an app-side scroll guard: the guard only masked the view-runaway and never cured the crash on WebKit. Verified on WebKit with no app-side mitigation: the previously crashing drags now pass, with controls asserting the selection really grew (67k chars) and the diff really was a clipped scroller.
|
Verified this fix against the original real-world reproduction (ThinkRail, Electrobun/WebKit desktop app embedding I ported the
To make sure these are real passes rather than a layout that lost its content, each run also asserts control metrics: the selection actually grew (~67k characters) and the diff was actually a clipped scroller ( This also retroactively explains why my standalone reproduction never crashed: I had removed We're shipping this as a vendored bun patch on our side until a release includes it. Thanks for the quick turnaround — happy to test any follow-up revision the same way. |
…ash (JetBrains#684) * fix(web): vendor the Pierre fix for the WebKit diff text-selection crash Drag-selecting text in a code diff crashed WebKit's web-content process; on the Electrobun desktop (macOS WKWebView) the workspace reloaded into "A problem repeatedly occurred". Root cause is upstream (pierrecomputer/pierre#1184): with the gutter utility enabled, Pierre's InteractionManager re-parented the utility button into the hovered line's number cell on every pointermove, so a native selection drag mutated the DOM under the in-progress selection and tripped a WebKit assert (EXC_BREAKPOINT in WebCore::EventHandler::handleMouseDraggedEvent). It is not virtualization and not the diff's grid layout: freezing Pierre's row rendering does not prevent it. Carry the upstream fix (pierrecomputer/pierre#1185) as a bun patch until a Pierre release ships it, instead of an app-side scroll guard: the guard only masked the view-runaway and never cured the crash on WebKit. Verified on WebKit with no app-side mitigation: the previously crashing drags now pass, with controls asserting the selection really grew (67k chars) and the diff really was a clipped scroller. * test(e2e): guard the diff gutter utility against moving during a text drag * docs(web): pin down the WebKit slot crash and the patch's silent-drop guard * test(e2e): wait for Pierre's highlighted render before the gutter-utility drag * fix(web): arm the Pierre drag guard on pointerleave and re-place the utility after the drag * build: invalidate turbo caches when a vendored patch changes * test(e2e): cover a drag that leaves the diff at once and a same-row release * docs(web): describe the extended Pierre patch and its two-test guard --------- Co-authored-by: danyaberezun <danya.berezun@gmail.com>
…after it Address two edge cases in the gutter-utility native-drag guard: - Arm `nativeTextDragActive` in `handlePointerLeave` (not only `handlePointerMove`). A native drag whose first sampled move lands outside the `<pre>` reaches `handlePointerLeave` with the button held; its `placeUtility()` would otherwise remove/reparent the utility under the active selection — the same WebKit crash this guards (pierre#1184). - Re-place the utility when the drag flag clears. When a drag ends on the same line as its last held-button move, `handlePointerEvent` sees `sameLine` and skips its own `placeUtility()`, leaving the button on the pre-drag row; re-place on the true->false transition so hover placement resumes immediately. Extracts the shared `isNativeTextDrag` predicate and extends the regression test to assert the utility follows hover again after a drag.
|
See my comments in the #1184 |
2 participants
Summary
Fixes the WebKit web-content process crash when drag-selecting text in a
FileDiffwith the review gutter utility enabled (enableGutterUtility), closing #1184.With
enableGutterUtility,InteractionManagerrepositioned the gutter utility button on everypointermoveby re-parenting it into the hovered line's number cell:During a native text-selection drag,
pointermovefires continuously while the selection auto-scrolls, so the utility node was moved between line cells under the in-progress native selection. WebKit asserts on that mid-drag DOM mutation and crashes (EXC_BREAKPOINTinWebCore::EventHandler::handleMouseDraggedEvent; Safari and WKWebView hosts such as Electrobun).Fix
Track a native text-selection drag — a mouse button held while no pierre gutter/line-selection session owns the pointer — via a new
nativeTextDragActiveflag, and makeplaceUtility()a no-op while it is set. The utility node is neither moved nor removed during the drag; placement resumes on the next hover move after the drag ends. The flag is armed inhandlePointerMoveand cleared on a freshhandlePointerDown.This is intentionally minimal and does not touch the diff layout. Note: an earlier hypothesis blamed
display: gridon the scroll container; that was verified not to be the cause (moving the grid off the scroll container still crashes with the gutter utility on), so no layout change is included.Testing
line-select.pw.ts: during a native text-selection drag the gutter utility stays on its original line instead of following the pointer. Fails before the fix (utility jumps to the drag's end line), passes after.moon run root:format root:lint— greenmoon run diffs:typecheck— greenmoon run diffs:test— 2140 pass / 0 failline-select,selection-action,annotations— 48 passCrash signature (confirmed)
~/Library/Logs/DiagnosticReports/com.apple.WebKit.WebContent.Development-*.ips,EXC_BREAKPOINT (SIGTRAP):