diff --git a/libraries/ranges-tracker/index.cjs b/libraries/ranges-tracker/index.cjs index 9ec9d44024..44370a4461 100644 --- a/libraries/ranges-tracker/index.cjs +++ b/libraries/ranges-tracker/index.cjs @@ -325,7 +325,7 @@ class RangesTracker { movedChanges.push(change) } else if (opStart === changeStart) { if ( - !(op.orderedRejections && alreadyMerged) && + !alreadyMerged && undoing && change.op.d.length >= op.i.length && change.op.d.slice(0, op.i.length) === op.i @@ -342,13 +342,11 @@ class RangesTracker { } alreadyMerged = true - if (op.orderedRejections) { - // Any tracked delete that came before this tracked delete - // rejection was moved after the incoming insert. Move them back - // so that they appear before the tracked delete rejection. - for (const trackedDelete of trackedDeletesAtOpPosition) { - trackedDelete.op.p -= opLength - } + // Any tracked delete that came before this tracked delete + // rejection was moved after the incoming insert. Move them back + // so that they appear before the tracked delete rejection. + for (const trackedDelete of trackedDeletesAtOpPosition) { + trackedDelete.op.p -= opLength } } else { // We're not rejecting that tracked delete. Move it after the @@ -656,8 +654,6 @@ class RangesTracker { _addOp(op, metadata) { // Don't take a reference to the existing op since we'll modify this in place with future changes op = this._clone(op) - // TODO: Remove this when the orderedRejections transition is over - delete op.orderedRejections const change = { id: this.newId(), op, diff --git a/libraries/ranges-tracker/test/unit/ranges-tracker-test.js b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js index 55d73f7b85..cd7be84a15 100644 --- a/libraries/ranges-tracker/test/unit/ranges-tracker-test.js +++ b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js @@ -136,62 +136,31 @@ describe('RangesTracker', function () { this.rangesTracker = new RangesTracker(this.changes, this.comments) }) - describe('with the orderedRejections flag', function () { - it('preserves the text order when rejecting changes', function () { - this.rangesTracker.applyOp( - { p: 50, i: 'this one', u: true, orderedRejections: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 33, d: 'before' } }, - { id: 'id2', op: { p: 50, d: 'right before' } }, - { id: 'id4', op: { p: 58, d: 'right after' } }, - { id: 'id5', op: { p: 83, d: 'long after' } }, - ]) - }) - - it('moves all tracked deletes after the insert if not rejecting changes', function () { - this.rangesTracker.applyOp( - { p: 50, i: 'some other text', u: true, orderedRejections: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 33, d: 'before' } }, - { id: 'id2', op: { p: 65, d: 'right before' } }, - { id: 'id3', op: { p: 65, d: 'this one' } }, - { id: 'id4', op: { p: 65, d: 'right after' } }, - { id: 'id5', op: { p: 90, d: 'long after' } }, - ]) - }) + it('preserves the text order when rejecting changes', function () { + this.rangesTracker.applyOp( + { p: 50, i: 'this one', u: true }, + { user_id: 'user-id' } + ) + expect(this.rangesTracker.changes).to.deep.equal([ + { id: 'id1', op: { p: 33, d: 'before' } }, + { id: 'id2', op: { p: 50, d: 'right before' } }, + { id: 'id4', op: { p: 58, d: 'right after' } }, + { id: 'id5', op: { p: 83, d: 'long after' } }, + ]) }) - describe('without the orderedRejections flag', function () { - it('puts the insert before tracked deletes when rejecting changes', function () { - this.rangesTracker.applyOp( - { p: 50, i: 'this one', u: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 33, d: 'before' } }, - { id: 'id2', op: { p: 58, d: 'right before' } }, - { id: 'id4', op: { p: 58, d: 'right after' } }, - { id: 'id5', op: { p: 83, d: 'long after' } }, - ]) - }) - - it('moves all tracked deletes after the insert if not rejecting changes', function () { - this.rangesTracker.applyOp( - { p: 50, i: 'some other text', u: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 33, d: 'before' } }, - { id: 'id2', op: { p: 65, d: 'right before' } }, - { id: 'id3', op: { p: 65, d: 'this one' } }, - { id: 'id4', op: { p: 65, d: 'right after' } }, - { id: 'id5', op: { p: 90, d: 'long after' } }, - ]) - }) + it('moves all tracked deletes after the insert if not rejecting changes', function () { + this.rangesTracker.applyOp( + { p: 50, i: 'some other text', u: true, orderedRejections: true }, + { user_id: 'user-id' } + ) + expect(this.rangesTracker.changes).to.deep.equal([ + { id: 'id1', op: { p: 33, d: 'before' } }, + { id: 'id2', op: { p: 65, d: 'right before' } }, + { id: 'id3', op: { p: 65, d: 'this one' } }, + { id: 'id4', op: { p: 65, d: 'right after' } }, + { id: 'id5', op: { p: 90, d: 'long after' } }, + ]) }) }) @@ -207,31 +176,16 @@ describe('RangesTracker', function () { this.rangesTracker = new RangesTracker(this.changes, this.comments) }) - describe('with the orderedRejections flag', function () { - it('removes only the first matching tracked delete', function () { - this.rangesTracker.applyOp( - { p: 10, i: 'giraffe', u: true, orderedRejections: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 10, d: 'cat' } }, - { id: 'id3', op: { p: 17, d: 'cat' } }, - { id: 'id4', op: { p: 17, d: 'giraffe' } }, - ]) - }) - }) - - describe('without the orderedRejections flag', function () { - it('removes all matching tracked delete', function () { - this.rangesTracker.applyOp( - { p: 10, i: 'giraffe', u: true }, - { user_id: 'user-id' } - ) - expect(this.rangesTracker.changes).to.deep.equal([ - { id: 'id1', op: { p: 17, d: 'cat' } }, - { id: 'id3', op: { p: 17, d: 'cat' } }, - ]) - }) + it('removes only the first matching tracked delete', function () { + this.rangesTracker.applyOp( + { p: 10, i: 'giraffe', u: true }, + { user_id: 'user-id' } + ) + expect(this.rangesTracker.changes).to.deep.equal([ + { id: 'id1', op: { p: 10, d: 'cat' } }, + { id: 'id3', op: { p: 17, d: 'cat' } }, + { id: 'id4', op: { p: 17, d: 'giraffe' } }, + ]) }) }) @@ -258,40 +212,20 @@ describe('RangesTracker', function () { this.rangesTracker = new RangesTracker(this.changes, this.comments) }) - describe('with the orderedRejections flag', function () { - it('places a tracked insert at the same position before both the delete and the insert', function () { - this.rangesTracker.track_changes = true - this.rangesTracker.applyOp( - { p: 10, i: 'incoming', orderedRejections: true }, - { user_id: 'user-id' } - ) - expect( - this.rangesTracker.changes.map(change => change.op) - ).to.deep.equal([ + it('places a tracked insert at the same position before both the delete and the insert', function () { + this.rangesTracker.track_changes = true + this.rangesTracker.applyOp( + { p: 10, i: 'incoming' }, + { user_id: 'user-id' } + ) + expect(this.rangesTracker.changes.map(change => change.op)).to.deep.equal( + [ { p: 5, d: 'before' }, { p: 10, i: 'incoming' }, { p: 18, d: 'delete' }, { p: 18, i: 'insert' }, - ]) - }) - }) - - describe('without the orderedRejections flag', function () { - it('places a tracked insert at the same position before both the delete and the insert', function () { - this.rangesTracker.track_changes = true - this.rangesTracker.applyOp( - { p: 10, i: 'incoming' }, - { user_id: 'user-id' } - ) - expect( - this.rangesTracker.changes.map(change => change.op) - ).to.deep.equal([ - { p: 5, d: 'before' }, - { p: 10, i: 'incoming' }, - { p: 18, d: 'delete' }, - { p: 18, i: 'insert' }, - ]) - }) + ] + ) }) }) }) diff --git a/services/web/frontend/js/vendor/libs/sharejs.js b/services/web/frontend/js/vendor/libs/sharejs.js index 2d200a16ff..ccd9113c69 100644 --- a/services/web/frontend/js/vendor/libs/sharejs.js +++ b/services/web/frontend/js/vendor/libs/sharejs.js @@ -698,7 +698,6 @@ export const { Doc } = (() => { op.u = true; // TODO: This flag is temporary. It is only necessary while we change // the behaviour of tracked delete rejections in RangesTracker - op.orderedRejections = true; op.fixedRemoveChange = true; } op = [op];