diff --git a/WORKLOG.md b/WORKLOG.md index 6c6b50e6d..11c964b08 100644 --- a/WORKLOG.md +++ b/WORKLOG.md @@ -2,6 +2,42 @@ ## 2026-09-07 +### quick-edit — stop RELOAD storms from cross-block index drift + +Debugged via da-live's `ew-editor-doc` collab-diagnostics branch (multi-user +test showed a collaborator's continuous edits pegging the main thread for +5-6s at a stretch, blocking local typing). Root cause traced to +`nx/public/plugins/quick-edit/src/prose.js`'s `createEditor`: it looked up +its target block by an exact `data-prose-index` match, but that index is a +global ProseMirror position — any edit *before* a block shifts it. +`handleTransaction` already re-shifts every other block's index for edits +inside an already-open mini-editor (`updateInstrumentation`), but +`createEditor` — the path taken the first time a block is touched by a +*remote* edit — never did, so the first remote edit to any not-yet-opened +block left every later block's cached index stale. Eventually some block's +`SET_EDITOR_STATE` arrived with a `cursorOffset` matching nothing, and the +portal gave up and asked the host to `RELOAD` (full body resend), which the +host answered unconditionally — no debounce — so a sustained editing burst +from one collaborator could retrigger this indefinitely. + +Fixed `createEditor` to fall back to `findTextBlock`'s existing +nearest-indexed-block lookup (`dom-index.js`) instead of giving up — the same +drift-tolerant match `findImageAtProseIndex` already relies on for images. +Added an `exclude` param to `findTextBlock`/`findNearestIndexed` so the +fallback can't resolve to (and destructively replace) a *different* block's +already-open `.prosemirror-editor`; the remote-cursor collaborator badge is +now only copied across on an exact match, not the fallback, so it can't get +misattributed to the wrong paragraph. Da-live also got a `quick-edit-controller.js` +RELOAD-coalescing debounce (150ms) as a stopgap while this was tracked down; +kept, since it's still a legitimate backstop. + +Verified via a two-browser test (da-nx files served through Chrome local +overrides): the RELOAD storm is gone under sustained multi-user editing. + +Review follow-ups: normalized `cursorOffset` to `Number` in `createEditor` so +the exact-match badge gate can't silently fail on a string, and added unit +tests for `findTextBlock`'s exclude + nearest-block fallback. + ### nx2/blocks/editortoggle — stop implicit `nx2:ew-user-enabled` writes on navigation `connectedCallback` used to reconcile the persisted `nx2:ew-user-enabled` flag diff --git a/nx/public/plugins/quick-edit/src/dom-index.js b/nx/public/plugins/quick-edit/src/dom-index.js index 91cdf88a8..b5b8d2dd7 100644 --- a/nx/public/plugins/quick-edit/src/dom-index.js +++ b/nx/public/plugins/quick-edit/src/dom-index.js @@ -12,12 +12,13 @@ export function safeQuerySelectorAll(root, selector) { } } -function findNearestIndexed(attr, from, root) { - const exact = root.querySelector(`[${attr}="${from}"]`); +function findNearestIndexed(attr, from, root, exclude) { + const suffix = exclude ? `:not(${exclude})` : ''; + const exact = root.querySelector(`[${attr}="${from}"]${suffix}`); if (exact) return exact; let best = null; let bestIndex = -1; - root.querySelectorAll(`[${attr}]`).forEach((el) => { + root.querySelectorAll(`[${attr}]${suffix}`).forEach((el) => { const idx = parseIndex(el.getAttribute(attr)); if (idx == null || idx > from) return; if (idx > bestIndex) { @@ -28,8 +29,10 @@ function findNearestIndexed(attr, from, root) { return best; } -export function findTextBlock(from, root = document) { - return findNearestIndexed('data-prose-index', from, root); +// exclude keeps an already-open editor out of the nearest-match fallback, so a +// drifted cursorOffset can't resolve to and replace a different block's editor. +export function findTextBlock(from, root = document, exclude = null) { + return findNearestIndexed('data-prose-index', from, root, exclude); } export function findBlock(from, root = document) { diff --git a/nx/public/plugins/quick-edit/src/prose.js b/nx/public/plugins/quick-edit/src/prose.js index 2d7b0c8d7..c8f46df20 100644 --- a/nx/public/plugins/quick-edit/src/prose.js +++ b/nx/public/plugins/quick-edit/src/prose.js @@ -13,6 +13,7 @@ import { createSimpleKeymap } from './simple-keymap.js'; import { createImageWrapperPlugin } from './image-wrapper.js'; import { setupImageDropListeners } from './images.js'; import { setRemoteCursors } from './cursors.js'; +import { findTextBlock } from './dom-index.js'; import { MESSAGE_TYPES } from '../../../../utils/message-types.js'; function marksEqual(a, b) { @@ -167,6 +168,9 @@ function keydown(view, event) { } function createEditor(cursorOffset, state, ctx) { + // Normalize once: the exact-match badge gate below is a strict === and would + // silently never match if cursorOffset arrived as a string. + const offset = Number(cursorOffset); const schema = getSchema(); const node = schema.nodeFromJSON(state); const doc = schema.node('doc', null, [node]); @@ -178,17 +182,22 @@ function createEditor(cursorOffset, state, ctx) { }); const editorParent = document.createElement('div'); - editorParent.setAttribute('data-prose-index', cursorOffset); + editorParent.setAttribute('data-prose-index', offset); editorParent.classList.add('prosemirror-editor'); - const element = document.querySelector(`[data-prose-index="${cursorOffset}"]`); + // Drift-tolerant lookup: an exact match can miss after another block's remote edit + // shifts positions. Exclude open editors so the fallback can't steal a live one. + const element = findTextBlock(offset, document, '.prosemirror-editor'); if (!element) { ctx.port.postMessage({ type: MESSAGE_TYPES.RELOAD }); return; } - if (element.getAttribute('data-cursor-remote')) { + // Only trust the found element's remote-cursor badge on an exact match — on the + // nearest-block fallback it belongs to whatever block drift landed on, not this one. + const isExactMatch = parseInt(element.getAttribute('data-prose-index'), 10) === offset; + if (isExactMatch && element.getAttribute('data-cursor-remote')) { editorParent.setAttribute('data-cursor-remote', element.getAttribute('data-cursor-remote')); editorParent.setAttribute('data-cursor-remote-color', element.getAttribute('data-cursor-remote-color')); } diff --git a/test/nx/public/plugins/quick-edit/dom-index.test.js b/test/nx/public/plugins/quick-edit/dom-index.test.js index 12aefce15..f0f4176cd 100644 --- a/test/nx/public/plugins/quick-edit/dom-index.test.js +++ b/test/nx/public/plugins/quick-edit/dom-index.test.js @@ -1,5 +1,38 @@ import { expect } from '@esm-bundle/chai'; -import { restoreBlockIndices } from '../../../../../nx/public/plugins/quick-edit/src/dom-index.js'; +import { restoreBlockIndices, findTextBlock } from '../../../../../nx/public/plugins/quick-edit/src/dom-index.js'; + +describe('findTextBlock', () => { + function root(html) { + const el = document.createElement('div'); + el.innerHTML = html; + return el; + } + + it('returns the exact data-prose-index match', () => { + const r = root('

a

b

'); + expect(findTextBlock(20, r)).to.equal(r.querySelector('[data-prose-index="20"]')); + }); + + it('falls back to the nearest block at-or-before a drifted offset', () => { + const r = root('

a

b

'); + // 23 has no exact match (positions drifted) — nearest at-or-before is 20. + expect(findTextBlock(23, r)).to.equal(r.querySelector('[data-prose-index="20"]')); + }); + + it('excludes an already-open editor from an exact match', () => { + const r = root('
live
' + + '

a

'); + // 20 matches the live editor exactly, but exclude must skip it and fall back to 10. + expect(findTextBlock(20, r, '.prosemirror-editor')).to.equal(r.querySelector('p')); + }); + + it('excludes an already-open editor from the nearest-block fallback', () => { + const r = root('

a

' + + '
live
'); + // Nearest at-or-before 23 is the live editor at 20; exclude it and use 10. + expect(findTextBlock(23, r, '.prosemirror-editor')).to.equal(r.querySelector('p')); + }); +}); describe('restoreBlockIndices', () => { it('stamps the authored variant from the source onto the live block', () => {