Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions WORKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions nx/public/plugins/quick-edit/src/dom-index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
anfibiacreativa marked this conversation as resolved.
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) {
Expand All @@ -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) {
Expand Down
15 changes: 12 additions & 3 deletions nx/public/plugins/quick-edit/src/prose.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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]);
Expand All @@ -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'));
}
Expand Down
35 changes: 34 additions & 1 deletion test/nx/public/plugins/quick-edit/dom-index.test.js
Original file line number Diff line number Diff line change
@@ -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('<p data-prose-index="10">a</p><p data-prose-index="20">b</p>');
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('<p data-prose-index="10">a</p><p data-prose-index="20">b</p>');
// 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('<div class="prosemirror-editor" data-prose-index="20">live</div>'
+ '<p data-prose-index="10">a</p>');
// 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('<p data-prose-index="10">a</p>'
+ '<div class="prosemirror-editor" data-prose-index="20">live</div>');
// 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', () => {
Expand Down
Loading