From a58550f23798d7dd4faa1a31ca9ca554e5a72161 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 8 Jun 2026 10:43:29 +0100 Subject: [PATCH 1/5] fix(editor): tag inserts with clientVars author until userAuthor propagates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the intermittent Firefox `clear_authorship_color` flake ("clear authorship colors can be undone to restore author colors"). Root cause: the inner editor's `thisAuthor` starts '' and is only populated when collab_client's `setProperty('userAuthor', userId)` reaches the inner frame — that call is queued by the outer ace wrapper via pendingInit until the iframe loads, so it is applied asynchronously. Under Firefox timing the first keystrokes can beat it, so freshly typed text is tagged author=''. An empty author canonicalizes to no-author, producing an unattributed insert (`Z:1>5+5$Hello`) that the server's pad-corruption guard rejects ("submitted an insert without an author attribute"), dropping the whole USER_CHANGES and losing authorship. Undo then cannot restore an author color and the test sees ``. Fix: a `getLocalAuthor()` helper that falls back to `clientVars.userId` (the same author id, available synchronously in the inner frame — already used as `myId` elsewhere) whenever `thisAuthor` is still empty, applied at the three new-text insert sites. The intentional clear-authorship path (`['author','']`) and the server-side guard are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 438f15fe2b3..6493f70ea2b 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -56,6 +56,17 @@ function Ace2Inner(editorInfo, cssManagers) { const SELECT_BUTTON_CLASS = 'selected'; let thisAuthor = ''; + // The local author id used to tag newly inserted text. `thisAuthor` is + // populated asynchronously: collab_client calls editor.setProperty('userAuthor', + // userId), which the outer ace wrapper queues via pendingInit until the inner + // iframe has loaded, then applies. Until that lands `thisAuthor` is '', and any + // text typed in that window would be tagged with an empty author. An empty + // author attribute canonicalizes to "no author", so the wire changeset is an + // unattributed insert — which the server's pad-corruption guard rejects, taking + // the whole USER_CHANGES with it and silently dropping authorship (the Firefox + // clear_authorship_color flake). clientVars.userId is the same author id and is + // available synchronously in the inner frame, so fall back to it. + const getLocalAuthor = () => thisAuthor || window.clientVars?.userId || ''; let disposed = false; const outerWin = document.getElementsByName("ace_outer")[0] @@ -183,7 +194,7 @@ function Ace2Inner(editorInfo, cssManagers) { buildKeepToStartOfRange(rep, builder, start); buildRemoveRange(rep, builder, start, end); builder.insert(newText, [ - ['author', thisAuthor], + ['author', getLocalAuthor()], ], rep.apool); const cs = builder.toString(); @@ -1309,7 +1320,7 @@ function Ace2Inner(editorInfo, cssManagers) { const cs = new Builder(rep.lines.totalWidth()).keep( rep.lines.offsetOfIndex(lineNum), lineNum).insert( theIndent, [ - ['author', thisAuthor], + ['author', getLocalAuthor()], ], rep.apool).toString(); performDocumentApplyChangeset(cs); performSelectionChange([lineNum, theIndent.length], [lineNum, theIndent.length]); @@ -1850,7 +1861,7 @@ function Ace2Inner(editorInfo, cssManagers) { let isNewTextMultiauthor = false; const authorizer = cachedStrFunc((oldAtts) => { const attribs = AttributeMap.fromString(oldAtts, rep.apool); - if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', thisAuthor); + if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', getLocalAuthor()); return attribs.toString(); }); From e44b6568e9cdca1fd7da2a08bc2f273d3b86cd43 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 8 Jun 2026 10:54:32 +0100 Subject: [PATCH 2/5] fix(editor): also seed line-attribute author to close the same race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first commit fixed text inserts, but line-attribute changes (lists, headings, alignment) emit a line-marker insert tagged with AttributeManager.author, which likewise starts '' and is only set when the async setProperty('userAuthor') lands. An early list/heading could therefore emit an unattributed line-marker insert and hit the same server-side rejection. Seed documentAttributeManager.author from getLocalAuthor() (clientVars fallback) right after it is constructed; the userauthor handler keeps it in sync after. Audited the other author-tagging paths: all ace2_inner text-insert sites go through getLocalAuthor()/authorizer now, and contentcollector (paste/import) derives authorship from the pasted DOM's author- classes via className2Author — a different mechanism, not this async race — and self-guards on `if (state.author)`. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 6493f70ea2b..3fa36e72c00 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -3831,6 +3831,13 @@ function Ace2Inner(editorInfo, cssManagers) { // Init documentAttributeManager documentAttributeManager = new AttributeManager(rep, performDocumentApplyChangeset); + // Seed the line-attribute author the same way as text inserts (see + // getLocalAuthor): AttributeManager.author also starts '' and is otherwise + // only set when the async setProperty('userAuthor') lands, so an early line + // attribute (list/heading/alignment) would emit an unattributed line-marker + // insert that the server's pad-corruption guard rejects. The userauthor + // handler keeps it in sync afterwards. + documentAttributeManager.author = getLocalAuthor(); editorInfo.ace_performDocumentApplyAttributesToRange = (...args) => documentAttributeManager.setAttributesOnRange(...args); From ea7c2218950249abdc3bd23f3700ed9018eec760 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 8 Jun 2026 11:46:01 +0100 Subject: [PATCH 3/5] fix(editor): read the local author from the TOP pad window, not the inner frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1 of this fix fell back to `window.clientVars?.userId`, but that did NOT resolve the flake — CI reproduced the identical failure and the server log still showed the unattributed-insert rejection. Ground truth via direct DOM measurement of a running pad: the inner editor iframe's `window.clientVars` is NEVER populated (undefined at t=0/1/3/6s for the life of the pad), so the v1 fallback always returned ''. The author id lives on the TOP pad window (`window.top.clientVars.userId`), set by pad.ts when the CLIENT_VARS message arrives — which necessarily precedes editor creation, so it is reliably available from the inner frame the moment the editor can accept input. getLocalAuthor() now reads window.top.clientVars.userId (guarded with try/catch for cross-origin embedded pads). thisAuthor still takes precedence once the queued setProperty('userAuthor') lands. All text-insert sites and the line-attribute seed already route through getLocalAuthor(), so both paths are covered. Verification: ts-check passes; fix source confirmed by direct measurement (the top window reliably holds the author at editor-init time). Local end-to-end repro was not possible — the dev server's require-kernel bundle did not reflect working-tree edits — so this is validated against the measured source + CI. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 3fa36e72c00..d776b62885a 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -64,9 +64,26 @@ function Ace2Inner(editorInfo, cssManagers) { // author attribute canonicalizes to "no author", so the wire changeset is an // unattributed insert — which the server's pad-corruption guard rejects, taking // the whole USER_CHANGES with it and silently dropping authorship (the Firefox - // clear_authorship_color flake). clientVars.userId is the same author id and is - // available synchronously in the inner frame, so fall back to it. - const getLocalAuthor = () => thisAuthor || window.clientVars?.userId || ''; + // clear_authorship_color flake). + // + // The inner editor iframe never receives its own `window.clientVars` (verified + // by direct measurement: it stays undefined for the life of the pad). The + // author id lives on the TOP pad window, set by pad.ts when the CLIENT_VARS + // message arrives — which is necessarily before the editor is interactive. Read + // it from there so an early insert always carries the correct author. Once the + // queued setProperty('userAuthor') lands, `thisAuthor` takes precedence. + const getLocalAuthor = () => { + if (thisAuthor) return thisAuthor; + try { + // @ts-ignore - clientVars is not typed on Window + return (window.top && window.top.clientVars && window.top.clientVars.userId) || ''; + } catch (e) { + // Cross-origin top (embedded pad): top is inaccessible. Fall back to any + // clientVars on this frame (may be '' for embeds, preserving prior behavior). + // @ts-ignore + return (window.clientVars && window.clientVars.userId) || ''; + } + }; let disposed = false; const outerWin = document.getElementsByName("ace_outer")[0] From 27ae8c8bd57bb6203adedee9bcb68376ebc26223 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 8 Jun 2026 13:24:44 +0100 Subject: [PATCH 4/5] fix(editor): walk ancestor frames for the author instead of window.top MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a correctness gap flagged in review: reading window.top.clientVars fails for same-origin embeds, where window.top is the host page (accessible, so no exception is thrown) and has no Etherpad clientVars — getLocalAuthor() then returns '' and the unattributed-insert bug recurs. Walk up the ancestor chain (inner -> ace_outer -> pad window) and return the first frame that has clientVars.userId. This stops at the pad window and never depends on window.top, so it is correct for normal pads, same-origin embeds, and cross-origin embeds (the try/catch ends the walk if an ancestor is cross-origin, though the pad window — always same-origin with the editor frames — is reached first). Behavior in the non-embed case is unchanged (pad window is 2 hops up). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index d776b62885a..e4921e7bbe7 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -68,21 +68,33 @@ function Ace2Inner(editorInfo, cssManagers) { // // The inner editor iframe never receives its own `window.clientVars` (verified // by direct measurement: it stays undefined for the life of the pad). The - // author id lives on the TOP pad window, set by pad.ts when the CLIENT_VARS - // message arrives — which is necessarily before the editor is interactive. Read - // it from there so an early insert always carries the correct author. Once the - // queued setProperty('userAuthor') lands, `thisAuthor` takes precedence. + // author id lives on the pad window (`pad.ts` sets `clientVars` there when the + // CLIENT_VARS message arrives, necessarily before the editor is interactive), + // which is an ancestor of this frame: inner -> ace_outer -> pad window. + // + // Walk UP the ancestor chain and return the first frame that actually has + // `clientVars.userId`. This stops at the pad window and never depends on + // `window.top`: in a same-origin embed `window.top` is the host page (which has + // no clientVars), so reading from `top` would wrongly yield '' and reintroduce + // the unattributed-insert bug. Guarded with try/catch so a cross-origin ancestor + // (cross-origin embed) ends the walk instead of throwing. Once the queued + // setProperty('userAuthor') lands, `thisAuthor` takes precedence. const getLocalAuthor = () => { if (thisAuthor) return thisAuthor; try { - // @ts-ignore - clientVars is not typed on Window - return (window.top && window.top.clientVars && window.top.clientVars.userId) || ''; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let w: any = window; + // Bounded to avoid an unexpected cycle; the pad window is 2 hops up. + for (let i = 0; i < 10 && w; i++) { + const id = w.clientVars && w.clientVars.userId; + if (id) return String(id); + if (w === w.parent) break; // reached the topmost accessible frame + w = w.parent; + } } catch (e) { - // Cross-origin top (embedded pad): top is inaccessible. Fall back to any - // clientVars on this frame (may be '' for embeds, preserving prior behavior). - // @ts-ignore - return (window.clientVars && window.clientVars.userId) || ''; + // Cross-origin ancestor (cross-origin embed): cannot read further up. } + return ''; }; let disposed = false; From ea0bb80bdfbf3e2ec0ddb1dc6c2999ae2bfd87a3 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 8 Jun 2026 14:13:28 +0100 Subject: [PATCH 5/5] fix(collab): stamp the author onto outgoing inserts in collab_client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the earlier getLocalAuthor attempts (v1-v3, reverted from ace2_inner.ts), which all failed because they tried to *read* an author that genuinely does not exist yet: the local author id (clientVars.userId) only arrives with the CLIENT_VARS socket message, and under load (Firefox + plugins) the editor can become editable and the user can type before that message lands. At that instant there is no author anywhere client-side, so the editor emits an unattributed insert that the server's pad-corruption guard rejects — dropping the change and losing authorship (the clear_authorship_color flake). Fix where the author IS reliably available: collab_client only exists after CLIENT_VARS has arrived, so its `userId` is always populated. A new pure helper stampAuthorOnInserts() rewrites the outgoing wire changeset so every '+' op carries an author, stamping userId onto any that lack one, just before it goes on the wire (both the USER_CHANGES send and the reconnect/further-changeset path). Independent of editor-init timing. Verification: - Unit test (stampAuthorOnInserts.test.ts, 5 cases): stamps the exact flake changeset `Z:1>5+5$Hello` -> authored + checkRep-valid + text preserved; leaves already-attributed inserts and keep/remove-only changesets unchanged; never invents an author when userId is empty; preserves other attributes. - ace2_inner.ts reverted to develop (the editor is unchanged; the fix is fully localized to the send path). - Local run of the clear_authorship_color spec with the fix: all pass, no server-side unattributed-insert rejections; full backend-new vitest green (the unrelated backend-tests-glob env-only failure aside). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 53 +------------ src/static/js/collab_client.ts | 22 +++++- src/static/js/stampAuthorOnInserts.ts | 58 ++++++++++++++ .../specs/stampAuthorOnInserts.test.ts | 76 +++++++++++++++++++ 4 files changed, 157 insertions(+), 52 deletions(-) create mode 100644 src/static/js/stampAuthorOnInserts.ts create mode 100644 src/tests/backend-new/specs/stampAuthorOnInserts.test.ts diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index e4921e7bbe7..438f15fe2b3 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -56,46 +56,6 @@ function Ace2Inner(editorInfo, cssManagers) { const SELECT_BUTTON_CLASS = 'selected'; let thisAuthor = ''; - // The local author id used to tag newly inserted text. `thisAuthor` is - // populated asynchronously: collab_client calls editor.setProperty('userAuthor', - // userId), which the outer ace wrapper queues via pendingInit until the inner - // iframe has loaded, then applies. Until that lands `thisAuthor` is '', and any - // text typed in that window would be tagged with an empty author. An empty - // author attribute canonicalizes to "no author", so the wire changeset is an - // unattributed insert — which the server's pad-corruption guard rejects, taking - // the whole USER_CHANGES with it and silently dropping authorship (the Firefox - // clear_authorship_color flake). - // - // The inner editor iframe never receives its own `window.clientVars` (verified - // by direct measurement: it stays undefined for the life of the pad). The - // author id lives on the pad window (`pad.ts` sets `clientVars` there when the - // CLIENT_VARS message arrives, necessarily before the editor is interactive), - // which is an ancestor of this frame: inner -> ace_outer -> pad window. - // - // Walk UP the ancestor chain and return the first frame that actually has - // `clientVars.userId`. This stops at the pad window and never depends on - // `window.top`: in a same-origin embed `window.top` is the host page (which has - // no clientVars), so reading from `top` would wrongly yield '' and reintroduce - // the unattributed-insert bug. Guarded with try/catch so a cross-origin ancestor - // (cross-origin embed) ends the walk instead of throwing. Once the queued - // setProperty('userAuthor') lands, `thisAuthor` takes precedence. - const getLocalAuthor = () => { - if (thisAuthor) return thisAuthor; - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let w: any = window; - // Bounded to avoid an unexpected cycle; the pad window is 2 hops up. - for (let i = 0; i < 10 && w; i++) { - const id = w.clientVars && w.clientVars.userId; - if (id) return String(id); - if (w === w.parent) break; // reached the topmost accessible frame - w = w.parent; - } - } catch (e) { - // Cross-origin ancestor (cross-origin embed): cannot read further up. - } - return ''; - }; let disposed = false; const outerWin = document.getElementsByName("ace_outer")[0] @@ -223,7 +183,7 @@ function Ace2Inner(editorInfo, cssManagers) { buildKeepToStartOfRange(rep, builder, start); buildRemoveRange(rep, builder, start, end); builder.insert(newText, [ - ['author', getLocalAuthor()], + ['author', thisAuthor], ], rep.apool); const cs = builder.toString(); @@ -1349,7 +1309,7 @@ function Ace2Inner(editorInfo, cssManagers) { const cs = new Builder(rep.lines.totalWidth()).keep( rep.lines.offsetOfIndex(lineNum), lineNum).insert( theIndent, [ - ['author', getLocalAuthor()], + ['author', thisAuthor], ], rep.apool).toString(); performDocumentApplyChangeset(cs); performSelectionChange([lineNum, theIndent.length], [lineNum, theIndent.length]); @@ -1890,7 +1850,7 @@ function Ace2Inner(editorInfo, cssManagers) { let isNewTextMultiauthor = false; const authorizer = cachedStrFunc((oldAtts) => { const attribs = AttributeMap.fromString(oldAtts, rep.apool); - if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', getLocalAuthor()); + if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', thisAuthor); return attribs.toString(); }); @@ -3860,13 +3820,6 @@ function Ace2Inner(editorInfo, cssManagers) { // Init documentAttributeManager documentAttributeManager = new AttributeManager(rep, performDocumentApplyChangeset); - // Seed the line-attribute author the same way as text inserts (see - // getLocalAuthor): AttributeManager.author also starts '' and is otherwise - // only set when the async setProperty('userAuthor') lands, so an early line - // attribute (list/heading/alignment) would emit an unattributed line-marker - // insert that the server's pad-corruption guard rejects. The userauthor - // handler keeps it in sync afterwards. - documentAttributeManager.author = getLocalAuthor(); editorInfo.ace_performDocumentApplyAttributesToRange = (...args) => documentAttributeManager.setAttributesOnRange(...args); diff --git a/src/static/js/collab_client.ts b/src/static/js/collab_client.ts index c90f92e80d3..bc5a533bb0b 100644 --- a/src/static/js/collab_client.ts +++ b/src/static/js/collab_client.ts @@ -26,6 +26,7 @@ const chat = require('./chat').chat; const hooks = require('./pluginfw/hooks'); const browser = require('./vendors/browser'); +import {stampAuthorOnInserts} from './stampAuthorOnInserts'; // Dependency fill on init. This exists for `pad.socket` only. // TODO: bind directly to the socket. @@ -48,6 +49,23 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) let commitDelay = 500; const userId = initialUserInfo.userId; + + // Build the outgoing changeset and guarantee every insert carries an author. + // collab_client only exists after CLIENT_VARS has arrived, so `userId` is always + // populated here — whereas the editor can build a changeset with an empty author + // if the user typed before CLIENT_VARS landed (the early-typing race that the + // server's pad-corruption guard rejects). Stamping here is the last line of + // defense before the changeset goes on the wire. See stampAuthorOnInserts. + const prepareUserChangeset = () => { + const data = editor.prepareUserChangeset(); + if (data.changeset) { + const stamped = stampAuthorOnInserts(data.changeset, data.apool, userId); + data.changeset = stamped.changeset; + data.apool = stamped.apool; + } + return data; + }; + // var socket; const userSet = {}; // userId -> userInfo userSet[userId] = initialUserInfo; @@ -114,7 +132,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) // Check if there are any pending revisions to be received from server. // Allow only if there are no pending revisions to be received from server if (!isPendingRevision) { - const userChangesData = editor.prepareUserChangeset(); + const userChangesData = prepareUserChangeset(); if (userChangesData.changeset) { lastCommitTime = now; committing = true; @@ -416,7 +434,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) obj.committedChangesetAPool = stateMessage.apool; editor.applyPreparedChangesetToBase(); } - const userChangesData = editor.prepareUserChangeset(); + const userChangesData = prepareUserChangeset(); if (userChangesData.changeset) { obj.furtherChangeset = userChangesData.changeset; obj.furtherChangesetAPool = userChangesData.apool; diff --git a/src/static/js/stampAuthorOnInserts.ts b/src/static/js/stampAuthorOnInserts.ts new file mode 100644 index 00000000000..a8f8afbf776 --- /dev/null +++ b/src/static/js/stampAuthorOnInserts.ts @@ -0,0 +1,58 @@ +'use strict'; + +import {deserializeOps, pack, unpack} from './Changeset'; +import AttributeMap from './AttributeMap'; +import AttributePool from './AttributePool'; +import {SmartOpAssembler} from './SmartOpAssembler'; + +/** + * Ensure every insert (`+`) op in a wire changeset carries an `author` attribute. + * + * Why this exists: the local author id (`clientVars.userId`) only becomes available + * once the CLIENT_VARS socket message arrives. Under load (notably Firefox with + * plugins) the editor can become editable and the user can type *before* that + * message lands, so the editor tags the insert with an empty author. An empty + * author canonicalizes to "no author", producing an unattributed insert that the + * server's pad-corruption guard rejects — dropping the whole USER_CHANGES and + * silently losing the typed text's authorship (the clear_authorship_color flake). + * + * This runs in collab_client, which only exists after CLIENT_VARS has arrived, so + * the author id passed here is always populated. Stamping any author-less insert + * just before the changeset is sent guarantees the server never sees an + * unattributed insert, independent of editor-init timing. + * + * @param changeset - The wire changeset string from prepareUserChangeset(). + * @param apoolJsonable - The jsonable wire attribute pool that accompanies it. + * @param authorId - The local author id (collab_client's userId). If falsy, the + * inputs are returned unchanged (nothing better to stamp with). + * @returns The (possibly) rewritten changeset + jsonable pool. Returns the inputs + * unchanged when no insert needed an author, so the common path is a no-op. + */ +export const stampAuthorOnInserts = ( + changeset: string, + apoolJsonable: any, + authorId: string, +): {changeset: string, apool: any} => { + if (!authorId) return {changeset, apool: apoolJsonable}; + const pool = (new AttributePool()).fromJsonable(apoolJsonable); + const unpacked = unpack(changeset); + const assem = new SmartOpAssembler(); + let modified = false; + for (const op of deserializeOps(unpacked.ops)) { + if (op.opcode === '+') { + const attribs = AttributeMap.fromString(op.attribs, pool); + if (!attribs.get('author')) { + attribs.set('author', authorId); + op.attribs = attribs.toString(); + modified = true; + } + } + assem.append(op); + } + if (!modified) return {changeset, apool: apoolJsonable}; + assem.endDocument(); + return { + changeset: pack(unpacked.oldLen, unpacked.newLen, assem.toString(), unpacked.charBank), + apool: pool.toJsonable(), + }; +}; diff --git a/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts b/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts new file mode 100644 index 00000000000..449814a2adc --- /dev/null +++ b/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts @@ -0,0 +1,76 @@ +'use strict'; + +import {describe, it, expect} from 'vitest'; +import {stampAuthorOnInserts} from '../../../static/js/stampAuthorOnInserts'; +import {checkRep, deserializeOps, unpack} from '../../../static/js/Changeset'; +import AttributeMap from '../../../static/js/AttributeMap'; +import AttributePool from '../../../static/js/AttributePool'; + +const AUTHOR = 'a.test1234567890'; +const EMPTY_POOL = () => (new AttributePool()).toJsonable(); + +// Read the author attribute off the first '+' op of a (changeset, jsonable pool). +const firstInsertAuthor = (changeset: string, apoolJsonable: any): string | undefined => { + const pool = (new AttributePool()).fromJsonable(apoolJsonable); + for (const op of deserializeOps(unpack(changeset).ops)) { + if (op.opcode === '+') return AttributeMap.fromString(op.attribs, pool).get('author'); + } + return undefined; +}; + +describe('stampAuthorOnInserts', () => { + it('stamps the author onto an unattributed insert (the flake changeset)', () => { + // `Z:1>5+5$Hello` — insert "Hello" with NO author, exactly what the editor + // emits during the early-typing race and what the server rejects. + const input = 'Z:1>5+5$Hello'; + const {changeset, apool} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR); + // The insert now carries the author... + expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR); + // ...the result is a valid canonical changeset... + expect(() => checkRep(changeset)).not.toThrow(); + // ...and the text is preserved. + expect(unpack(changeset).charBank).toBe('Hello'); + // It actually changed (was unattributed before). + expect(changeset).not.toBe(input); + }); + + it('leaves an already-attributed insert unchanged', () => { + // Build `Z:1>5*0+5$Hello` with author already in the pool. + const pool = new AttributePool(); + const n = pool.putAttrib(['author', AUTHOR]); // index 0 + const attributed = `Z:1>5*${n}+5$Hello`; + const jsonable = pool.toJsonable(); + const {changeset, apool} = stampAuthorOnInserts(attributed, jsonable, AUTHOR); + expect(changeset).toBe(attributed); // unchanged (no-op path) + expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR); + }); + + it('does not invent an author when authorId is empty', () => { + const input = 'Z:1>5+5$Hello'; + const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), ''); + expect(changeset).toBe(input); // nothing to stamp with → unchanged + }); + + it('does not touch keep/remove-only changesets', () => { + // `Z:6<1=5-1$` — keep 5, remove 1; no insert ops. + const input = 'Z:6<1=5-1$x'; + const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR); + expect(changeset).toBe(input); + }); + + it('preserves a non-author attribute already on the insert while adding author', () => { + // Insert with a bold attribute but no author. + const pool = new AttributePool(); + const b = pool.putAttrib(['bold', 'true']); // index 0 + const input = `Z:1>5*${b}+5$Hello`; + const {changeset, apool} = stampAuthorOnInserts(input, pool.toJsonable(), AUTHOR); + const outPool = (new AttributePool()).fromJsonable(apool); + let amap: AttributeMap | null = null; + for (const op of deserializeOps(unpack(changeset).ops)) { + if (op.opcode === '+') { amap = AttributeMap.fromString(op.attribs, outPool); break; } + } + expect(amap!.get('author')).toBe(AUTHOR); + expect(amap!.get('bold')).toBe('true'); + expect(() => checkRep(changeset)).not.toThrow(); + }); +});