Browse Source

Merge pull request #23993 from overleaf/em-remove-fixed-remove-change-flag

Remove fixedRemoveChange flag from editor updates

GitOrigin-RevId: bf74e1137560184c4b024a3b5c6ede5a841d3559
Eric Mc Sween 1 year ago
parent
commit
290bdf4361

+ 3 - 22
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)

+ 1 - 7
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([])
     })

+ 0 - 3
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];