Fix drag preview revert wedging further drags (#1296) - #1298
Merged
Conversation
revertPreview replayed a ChangeSet captured when the preview was inserted. If the document length moved on before the revert, ChangeSet.apply threw, and because lastDragPos was cleared only after the dispatch, every later dragover and drop re-threw with the same stale change set until reload. This is the top error by volume in Sentry. Three changes, none dependent on which path trips it: - Clear the stashed drag position before dispatching the revert, so one failure cannot wedge later drags. - Map the preview undo through any document changes made while the preview is showing, and skip the revert if the length still doesn't match, so an edit under the preview no longer throws. - Revert a leftover preview when the drag ends by any route, not only via dragleave/drop on the editor. Clearing the drag context (dragend) now reverts the preview and removes the stuck cm-drag-in-progress class, covering a drag cancelled or dropped elsewhere. Reworked the bare domEventHandlers into a ViewPlugin so the handlers can observe document updates and clean up on view destroy. Added two e2e cases: a document change during a drag, and a drag that ends without a dragleave. Both fail on the previous code. Not reproduced manually. The precondition and fix were worked out from the Sentry stack and the code; the mechanism was exercised via emulated drags under Playwright, not a real pointer, and not in Firefox where Sentry saw it. The e2e tests pin the behaviour going forward.
|
Preview build will be at |
Collaborator
Author
|
I've spent quite a while trying to figure out a repro for this one that doesn't rely on synthetic drag events without success. The logic holds up but how either of the scenarios identified above happens I do not know. I suspect it's the missing event rather than the doc change which I can't see a source for (at least for length, maybe a diagnostic could arrive late). |
microbit-robert
approved these changes
Sep 4, 2026
microbit-robert
left a comment
microbit-robert
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Code changes look good. I can't seem to break anything via drag and drop.
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
2 participants
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.
revertPreview replayed a ChangeSet captured when the preview was inserted. If the document length moved on before the revert, ChangeSet.apply threw, and because lastDragPos was cleared only after the dispatch, every later dragover and drop re-threw with the same stale change set until reload. This is the top error by volume in Sentry.
Three changes, none dependent on which path trips it:
Reworked the bare domEventHandlers into a ViewPlugin so the handlers can observe document updates and clean up on view destroy.
Added two e2e cases: a document change during a drag, and a drag that ends without a dragleave. Both fail on the previous code.
Not reproduced manually. The precondition and fix were worked out from the Sentry stack and the code; the mechanism was exercised via emulated drags under Playwright, not a real pointer (Firefox + Chrome).
Fixes #1296