From 77952e5d21f4c376c496b424254e6b2bee3d35b0 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Tue, 7 Apr 2026 08:46:52 -0400 Subject: [PATCH] Merge pull request #32628 from overleaf/em-bg-history-bug Prevent incorrect merging of ops with different hpos offsets GitOrigin-RevId: d171a93dcb29e952c7105afd81c3b29ca7e9788e --- .../app/js/UpdateCompressor.js | 10 +++ .../UpdateCompressor/UpdateCompressorTests.js | 90 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js index 5ae7591a7f..166a728cde 100644 --- a/services/project-history/app/js/UpdateCompressor.js +++ b/services/project-history/app/js/UpdateCompressor.js @@ -394,6 +394,16 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) { firstOp.p <= secondOp.p && secondOp.p <= firstOp.p + firstSize const combinedLengthUnderLimit = firstSize + secondSize < MAX_UPDATE_SIZE + // When ops come from a multi-component update, the history position offset + // (hpos - p) may differ between ops because each component's hpos is computed + // against a different tracked-change state. Merging ops with different + // offsets would produce incorrect history positions, so we bail out. + const firstHposOffset = (firstOp.hpos ?? firstOp.p) - firstOp.p + const secondHposOffset = (secondOp.hpos ?? secondOp.p) - secondOp.p + if (firstHposOffset !== secondHposOffset) { + return [firstUpdate, secondUpdate] + } + // Two inserts if ( firstOp.i != null && diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js index c8d50b16f4..4bb031de64 100644 --- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js +++ b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js @@ -877,6 +877,36 @@ describe('UpdateCompressor', function () { ]) }) + it('should not merge inserts with different hpos offsets (multi-component update)', function () { + // hpos offsets: first op has offset 10 (13-3), + // second op has offset 12 (18-6) + expect( + this.UpdateCompressor.compressUpdates([ + { + op: { p: 3, i: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 6, i: 'bar', hpos: 18 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + ).to.deep.equal([ + { + op: { p: 3, i: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 6, i: 'bar', hpos: 18 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + }) + it('should not merge updates from different users', function () { expect( this.UpdateCompressor.compressUpdates([ @@ -1091,6 +1121,36 @@ describe('UpdateCompressor', function () { ]) }) + it('should not merge deletes with different hpos offsets (multi-component update)', function () { + // hpos offsets: first op has offset 10 (13-3), + // second op has offset 12 (15-3) + expect( + this.UpdateCompressor.compressUpdates([ + { + op: { p: 3, d: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 3, d: 'bar', hpos: 15 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + ).to.deep.equal([ + { + op: { p: 3, d: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 3, d: 'bar', hpos: 15 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + }) + it('should not merge when the deletes are tracked', function () { // TODO: We should be able to lift that constraint, but it would // require recalculating the hpos on the second op. @@ -1264,6 +1324,36 @@ describe('UpdateCompressor', function () { }, ]) }) + + it('should not merge insert and delete with different hpos offsets (multi-component update)', function () { + // hpos offsets: first op has offset 10 (13-3), + // second op has offset 12 (17-5) + expect( + this.UpdateCompressor.compressUpdates([ + { + op: { p: 3, i: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 5, d: 'o', hpos: 17 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + ).to.deep.equal([ + { + op: { p: 3, i: 'foo', hpos: 13 }, + meta: { ts: this.ts1, user_id: this.user_id }, + v: 42, + }, + { + op: { p: 5, d: 'o', hpos: 17 }, + meta: { ts: this.ts2, user_id: this.user_id }, + v: 43, + }, + ]) + }) }) describe('delete - insert', function () {