diff --git a/src/static/js/broadcast.ts b/src/static/js/broadcast.ts index 347cf29e2..01e4a2edf 100644 --- a/src/static/js/broadcast.ts +++ b/src/static/js/broadcast.ts @@ -26,7 +26,7 @@ const makeCSSManager = require('./cssmanager').makeCSSManager; const domline = require('./domline').domline; import AttribPool from './AttributePool'; -import {compose, deserializeOps, inverse, moveOpsToNewPool, mutateAttributionLines, mutateTextLines, splitAttributionLines, splitTextLines, unpack} from './Changeset'; +import {compose, deserializeOps, inverse, isIdentity, moveOpsToNewPool, mutateAttributionLines, mutateTextLines, splitAttributionLines, splitTextLines, unpack} from './Changeset'; const attributes = require('./attributes'); const linestylefilter = require('./linestylefilter').linestylefilter; const colorutils = require('./colorutils').colorutils; @@ -139,52 +139,59 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro BroadcastSlider.setSliderPosition(revision); } - const oldAlines = padContents.alines.slice(); - try { - // must mutate attribution lines before text lines - mutateAttributionLines(changeset, padContents.alines, padContents.apool); - } catch (e) { - debugLog(e); - } - - // scroll to the area that is changed before the lines are mutated - if ($('#options-followContents').is(':checked') || - $('#options-followContents').prop('checked')) { - // get the index of the first line that has mutated attributes - // the last line in `oldAlines` should always equal to "|1+1", ie newline without attributes - // so it should be safe to assume this line has changed attributes when inserting content at - // the bottom of a pad - let lineChanged; - _.some(oldAlines, (line, index) => { - if (line !== padContents.alines[index]) { - lineChanged = index; - return true; // break - } - }); - // some chars are replaced (no attributes change and no length change) - // test if there are keep ops at the start of the cs - if (lineChanged === undefined) { - const [op] = deserializeOps(unpack(changeset).ops); - lineChanged = op != null && op.opcode === '=' ? op.lines : 0; + // Skip mutation for identity changesets (no actual change), but still advance + // revision/time state. Identity changesets can appear when compose() of multiple + // revisions produces a net-zero change, or from import/save sequences. + // See https://github.com/ether/etherpad-lite/issues/5214 + if (!isIdentity(changeset)) { + const oldAlines = padContents.alines.slice(); + try { + // must mutate attribution lines before text lines + mutateAttributionLines(changeset, padContents.alines, padContents.apool); + } catch (e) { + debugLog(e); } - const goToLineNumber = (lineNumber) => { - // Sets the Y scrolling of the browser to go to this line - const line = $('#innerdocbody').find(`div:nth-child(${lineNumber + 1})`); - const newY = $(line)[0].offsetTop; - const ecb = document.getElementById('editorcontainerbox'); - // Chrome 55 - 59 bugfix - if (ecb.scrollTo) { - ecb.scrollTo({top: newY, behavior: 'auto'}); - } else { - $('#editorcontainerbox').scrollTop(newY); + // scroll to the area that is changed before the lines are mutated + if ($('#options-followContents').is(':checked') || + $('#options-followContents').prop('checked')) { + // get the index of the first line that has mutated attributes + // the last line in `oldAlines` should always equal to "|1+1", ie newline without attributes + // so it should be safe to assume this line has changed attributes when inserting content at + // the bottom of a pad + let lineChanged; + _.some(oldAlines, (line, index) => { + if (line !== padContents.alines[index]) { + lineChanged = index; + return true; // break + } + }); + // some chars are replaced (no attributes change and no length change) + // test if there are keep ops at the start of the cs + if (lineChanged === undefined) { + const [op] = deserializeOps(unpack(changeset).ops); + lineChanged = op != null && op.opcode === '=' ? op.lines : 0; } - }; - goToLineNumber(lineChanged); + const goToLineNumber = (lineNumber) => { + // Sets the Y scrolling of the browser to go to this line + const line = $('#innerdocbody').find(`div:nth-child(${lineNumber + 1})`); + const newY = $(line)[0].offsetTop; + const ecb = document.getElementById('editorcontainerbox'); + // Chrome 55 - 59 bugfix + if (ecb.scrollTo) { + ecb.scrollTo({top: newY, behavior: 'auto'}); + } else { + $('#editorcontainerbox').scrollTop(newY); + } + }; + + goToLineNumber(lineChanged); + } + + mutateTextLines(changeset, padContents); } - mutateTextLines(changeset, padContents); padContents.currentRevision = revision; padContents.currentTime += timeDelta; @@ -201,7 +208,9 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro revisionInfo.addChangeset( revision, revision + 1, changesetForward, changesetBackward, timeDelta); BroadcastSlider.setSliderLength(revisionInfo.latest); - if (broadcasting) applyChangeset(changesetForward, revision + 1, false, timeDelta); + if (broadcasting) { + applyChangeset(changesetForward, revision + 1, false, timeDelta); + } }; /* @@ -276,7 +285,9 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro changeset = compose(changeset, cs[i], padContents.apool); timeDelta += path.times[i]; } - if (changeset) applyChangeset(changeset, path.rev, true, timeDelta); + if (changeset) { + applyChangeset(changeset, path.rev, true, timeDelta); + } } else if (path.status === 'partial') { // callback is called after changeset information is pulled from server // this may never get called, if the changeset has already been loaded @@ -294,7 +305,9 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro changeset = compose(changeset, cs[i], padContents.apool); timeDelta += path.times[i]; } - if (changeset) applyChangeset(changeset, path.rev, true, timeDelta); + if (changeset) { + applyChangeset(changeset, path.rev, true, timeDelta); + } // Loading changeset history for new revision loadChangesetsForRevision(newRevision, update); diff --git a/src/tests/frontend-new/specs/timeslider_identity_changeset.spec.ts b/src/tests/frontend-new/specs/timeslider_identity_changeset.spec.ts new file mode 100644 index 000000000..8e7c4f629 --- /dev/null +++ b/src/tests/frontend-new/specs/timeslider_identity_changeset.spec.ts @@ -0,0 +1,103 @@ +import {expect, test} from "@playwright/test"; +import {goToNewPad, getPadBody, clearPadContent, writeToPad} from "../helper/padHelper"; + +/** + * Regression test for https://github.com/ether/etherpad-lite/issues/5214 + * + * When a pad's revision history contains an identity changeset (Z:N>0$, + * representing no actual change), the timeslider playback would crash or + * break because the broadcast code tried to apply it as a real change. + * + * Identity changesets appear when compose() of multiple revisions produces + * a net-zero change (e.g., type "hello" then delete "hello"). + */ +test.describe('Timeslider with identity changesets (bug #5214)', function () { + + test('timeslider playback advances through all revisions including identity changesets', async function ({page}) { + // Create a pad with several revisions to exercise the timeslider + const padId = await goToNewPad(page); + const body = await getPadBody(page); + await body.click(); + await clearPadContent(page); + + // Create multiple revisions by typing, deleting, retyping. + // When compose() composes the delete+retype, it can produce an identity changeset. + await writeToPad(page, 'First revision text'); + await page.waitForTimeout(500); + + // Select all and delete (creates a "delete everything" revision) + await page.keyboard.down('Control'); + await page.keyboard.press('A'); + await page.keyboard.up('Control'); + await page.keyboard.press('Backspace'); + await page.waitForTimeout(500); + + // Type new content (combined with delete above, compose can yield identity) + await writeToPad(page, 'After delete'); + await page.waitForTimeout(1000); + + // Navigate to timeslider at revision 0 so playback has revisions to advance through + await page.goto(`http://localhost:9001/p/${padId}/timeslider#0`); + await page.waitForSelector('#timeslider-slider', {timeout: 10000}); + await page.waitForTimeout(1000); + + // Click play to start playback from rev 0 + await page.locator('#playpause_button_icon').click(); + + // Wait for playback to progress through revisions + await page.waitForTimeout(5000); + + // The page should not have crashed — check for error gritter popups + const errors = page.locator('.gritter-item.error'); + expect(await errors.count()).toBe(0); + + // The timeslider should still be functional + await expect(page.locator('#timeslider-slider')).toBeVisible(); + }); + + test('timeslider can scrub through all revisions without error', async function ({page}) { + const padId = await goToNewPad(page); + const body = await getPadBody(page); + await body.click(); + await clearPadContent(page); + + // Create revisions + await writeToPad(page, 'Hello'); + await page.waitForTimeout(300); + await writeToPad(page, ' World'); + await page.waitForTimeout(300); + + // Select all and delete + await page.keyboard.down('Control'); + await page.keyboard.press('A'); + await page.keyboard.up('Control'); + await page.keyboard.press('Backspace'); + await page.waitForTimeout(300); + + // Retype + await writeToPad(page, 'New text'); + await page.waitForTimeout(1000); + + // Go to timeslider + await page.goto(`http://localhost:9001/p/${padId}/timeslider`); + await page.waitForSelector('#timeslider-slider', {timeout: 10000}); + + // Get the total number of revisions from the slider + const sliderLength = await page.evaluate(() => { + return (window as any).BroadcastSlider?.getSliderLength?.() ?? 0; + }); + + // Scrub through each revision from 0 to latest + for (let rev = 0; rev <= sliderLength; rev++) { + await page.goto(`http://localhost:9001/p/${padId}/timeslider#${rev}`); + await page.waitForTimeout(300); + + // Check no errors appeared + const errors = page.locator('.gritter-item.error'); + expect(await errors.count()).toBe(0); + } + + // Final check: timeslider is still functional + await expect(page.locator('#timeslider-slider')).toBeVisible(); + }); +});