From 4c32ee038ad240ff4eafeba3ef72f9244a8972b9 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:48:36 -0500 Subject: [PATCH] Merge pull request #23993 from overleaf/em-remove-fixed-remove-change-flag Remove fixedRemoveChange flag from editor updates GitOrigin-RevId: bf74e1137560184c4b024a3b5c6ede5a841d3559 --- libraries/ranges-tracker/index.cjs | 25 +++---------------- .../test/unit/ranges-tracker-test.js | 8 +----- .../web/frontend/js/vendor/libs/sharejs.js | 3 --- 3 files changed, 4 insertions(+), 32 deletions(-) diff --git a/libraries/ranges-tracker/index.cjs b/libraries/ranges-tracker/index.cjs index 44370a4461..a94e265e80 100644 --- a/libraries/ranges-tracker/index.cjs +++ b/libraries/ranges-tracker/index.cjs @@ -306,7 +306,6 @@ class RangesTracker { const opLength = op.i.length const opEnd = op.p + opLength const undoing = !!op.u - const fixedRemoveChange = op.fixedRemoveChange let alreadyMerged = false let previousChange = null @@ -472,11 +471,7 @@ class RangesTracker { } for (change of removeChanges) { - if (fixedRemoveChange) { - this._removeChange(change) - } else { - this._brokenRemoveChange(change) - } + this._removeChange(change) } for (change of movedChanges) { @@ -489,7 +484,6 @@ class RangesTracker { const opLength = op.d.length const opEnd = op.p + opLength const removeChanges = [] - const fixedRemoveChange = op.fixedRemoveChange let movedChanges = [] // We might end up modifying our delete op if it merges with existing deletes, or cancels out @@ -617,11 +611,7 @@ class RangesTracker { movedChanges.push(change) op.d = '' // stop it being added } else { - if (fixedRemoveChange) { - this._removeChange(change) - } else { - this._brokenRemoveChange(change) - } + this._removeChange(change) } } @@ -637,11 +627,7 @@ class RangesTracker { const results = this._scanAndMergeAdjacentUpdates() movedChanges = movedChanges.concat(results.movedChanges) for (const change of results.removeChanges) { - if (fixedRemoveChange) { - this._removeChange(change) - } else { - this._brokenRemoveChange(change) - } + this._removeChange(change) movedChanges = movedChanges.filter(c => c !== change) } } @@ -683,11 +669,6 @@ class RangesTracker { this._markAsDirty(change, 'change', 'removed') } - _brokenRemoveChange(change) { - this.changes = this.changes.filter(c => c.id !== change.id) - this._markAsDirty(change, 'change', 'removed') - } - _applyOpModifications(content, opModifications) { // Put in descending position order, with deleting first if at the same offset // (Inserting first would modify the content that the delete will delete) diff --git a/libraries/ranges-tracker/test/unit/ranges-tracker-test.js b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js index cd7be84a15..d20734962f 100644 --- a/libraries/ranges-tracker/test/unit/ranges-tracker-test.js +++ b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js @@ -40,7 +40,7 @@ describe('RangesTracker', function () { }) it("deleting one tracked insert doesn't delete the others", function () { - this.rangesTracker.applyOp({ p: 20, d: 'two', fixedRemoveChange: true }) + this.rangesTracker.applyOp({ p: 20, d: 'two' }) expect(this.rangesTracker.changes).to.deep.equal([ this.changes[0], this.changes[2], @@ -64,7 +64,6 @@ describe('RangesTracker', function () { this.rangesTracker.applyOp({ p: 15, d: '567890123456789012345', - fixedRemoveChange: true, }) expect(this.rangesTracker.changes.map(c => c.op)).to.deep.equal([ { p: 10, d: 'one' }, @@ -77,7 +76,6 @@ describe('RangesTracker', function () { this.rangesTracker.applyOp({ p: 20, d: '0123456789', - fixedRemoveChange: true, }) expect(this.rangesTracker.changes.map(c => c.op)).to.deep.equal([ { p: 10, d: 'one' }, @@ -91,7 +89,6 @@ describe('RangesTracker', function () { p: 20, i: 'two', u: true, - fixedRemoveChange: true, }) expect(this.rangesTracker.changes.map(c => c.op)).to.deep.equal([ { p: 10, d: 'one' }, @@ -105,19 +102,16 @@ describe('RangesTracker', function () { p: 10, i: 'one', u: true, - fixedRemoveChange: true, }) this.rangesTracker.applyOp({ p: 23, i: 'two', u: true, - fixedRemoveChange: true, }) this.rangesTracker.applyOp({ p: 36, i: 'three', u: true, - fixedRemoveChange: true, }) expect(this.rangesTracker.changes.map(c => c.op)).to.deep.equal([]) }) diff --git a/services/web/frontend/js/vendor/libs/sharejs.js b/services/web/frontend/js/vendor/libs/sharejs.js index ccd9113c69..ffaf0ea4fb 100644 --- a/services/web/frontend/js/vendor/libs/sharejs.js +++ b/services/web/frontend/js/vendor/libs/sharejs.js @@ -696,9 +696,6 @@ export const { Doc } = (() => { var op = { p: pos, i: text }; if (fromUndo) { op.u = true; - // TODO: This flag is temporary. It is only necessary while we change - // the behaviour of tracked delete rejections in RangesTracker - op.fixedRemoveChange = true; } op = [op];