From f2d08f913b88d4f18627a9e1cae9e66a72293010 Mon Sep 17 00:00:00 2001 From: Christian Williams Date: Wed, 10 Oct 2012 11:08:33 -0700 Subject: [PATCH] Added reversed boolean for translateOffset Previously, Undo used a transaction's lengthDifference to calculate the selection to display after the transaction was undone. Now, translateOffset with the reversed boolean set to true will properly translate the inverse of a transaction's selection change. Fixes bug #40538 Change-Id: I110bc0cbb5824547842efd391b9f2948b037b758 --- modules/ve/dm/ve.dm.Surface.js | 33 ++++++++--------- modules/ve/dm/ve.dm.Transaction.js | 20 +++++----- modules/ve/test/dm/ve.dm.Transaction.test.js | 39 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js index 2877a2e9ad..0827ea962c 100644 --- a/modules/ve/dm/ve.dm.Surface.js +++ b/modules/ve/dm/ve.dm.Surface.js @@ -173,19 +173,20 @@ ve.dm.Surface.prototype.breakpoint = function ( selection ) { }; ve.dm.Surface.prototype.undo = function () { - var diff, item, i, selection; + var item, i, transaction, selection; this.breakpoint(); this.undoIndex++; + if ( this.bigStack[this.bigStack.length - this.undoIndex] ) { this.emit( 'lock' ); - diff = 0; item = this.bigStack[this.bigStack.length - this.undoIndex]; - for ( i = item.stack.length - 1; i >= 0; i-- ) { - this.documentModel.rollback( item.stack[i] ); - diff += item.stack[i].lengthDifference; - } selection = item.selection; - selection.end -= diff; + + for ( i = item.stack.length - 1; i >= 0; i-- ) { + transaction = item.stack[i]; + selection = transaction.translateRange( selection, true ); + this.documentModel.rollback( item.stack[i] ); + } this.emit( 'unlock' ); this.emit( 'history' ); return selection; @@ -194,19 +195,15 @@ ve.dm.Surface.prototype.undo = function () { }; ve.dm.Surface.prototype.redo = function () { - var selection, diff, item, i; + var selection, item, i; this.breakpoint(); - if ( this.undoIndex > 0 ) { + + if ( this.undoIndex > 0 && this.bigStack[this.bigStack.length - this.undoIndex] ) { this.emit( 'lock' ); - if ( this.bigStack[this.bigStack.length - this.undoIndex] ) { - diff = 0; - item = this.bigStack[this.bigStack.length - this.undoIndex]; - for ( i = 0; i < item.stack.length; i++ ) { - this.documentModel.commit( item.stack[i] ); - diff += item.stack[i].lengthDifference; - } - selection = item.selection; - selection.end += diff; + item = this.bigStack[this.bigStack.length - this.undoIndex]; + selection = item.selection; + for ( i = 0; i < item.stack.length; i++ ) { + this.documentModel.commit( item.stack[i] ); } this.undoIndex--; this.emit( 'unlock' ); diff --git a/modules/ve/dm/ve.dm.Transaction.js b/modules/ve/dm/ve.dm.Transaction.js index 965f4d8d62..499cac1c5d 100644 --- a/modules/ve/dm/ve.dm.Transaction.js +++ b/modules/ve/dm/ve.dm.Transaction.js @@ -495,20 +495,22 @@ ve.dm.Transaction.prototype.getLengthDifference = function () { * @param {Number} offset Offset in the linear model before the transaction has been processed * @returns {Number} Translated offset, as it will be after processing transaction */ -ve.dm.Transaction.prototype.translateOffset = function ( offset ) { - var i, cursor = 0, adjustment = 0, op; +ve.dm.Transaction.prototype.translateOffset = function ( offset, reversed ) { + var i, cursor = 0, adjustment = 0, op, insertLength, removeLength; for ( i = 0; i < this.operations.length; i++ ) { op = this.operations[i]; if ( op.type === 'replace' ) { - adjustment += op.insert.length - op.remove.length; - if ( offset === cursor + op.remove.length ) { + insertLength = reversed ? op.remove.length : op.insert.length; + removeLength = reversed ? op.insert.length : op.remove.length; + adjustment += insertLength - removeLength; + if ( offset === cursor + removeLength ) { // Offset points to right after the removal, translate it return offset + adjustment; - } else if ( offset >= cursor && offset < cursor + op.remove.length ) { + } else if ( offset >= cursor && offset < cursor + removeLength ) { // The offset points inside of the removal - return cursor + op.remove.length + adjustment; + return cursor + removeLength + adjustment; } - cursor += op.remove.length; + cursor += removeLength; } else if ( op.type === 'retain' ) { if ( offset >= cursor && offset < cursor + op.length ) { return offset + adjustment; @@ -530,8 +532,8 @@ ve.dm.Transaction.prototype.translateOffset = function ( offset ) { * @param {ve.Range} range Range in the linear model before the transaction has been processed * @returns {ve.Range} Translated range, as it will be after processing transaction */ -ve.dm.Transaction.prototype.translateRange = function ( range ) { - return new ve.Range( this.translateOffset( range.from ), this.translateOffset( range.to ) ); +ve.dm.Transaction.prototype.translateRange = function ( range, reversed ) { + return new ve.Range( this.translateOffset( range.from, reversed ), this.translateOffset( range.to, reversed ) ); }; /** diff --git a/modules/ve/test/dm/ve.dm.Transaction.test.js b/modules/ve/test/dm/ve.dm.Transaction.test.js index 652d10f9c0..660a12310c 100644 --- a/modules/ve/test/dm/ve.dm.Transaction.test.js +++ b/modules/ve/test/dm/ve.dm.Transaction.test.js @@ -805,6 +805,45 @@ QUnit.test( 'translateOffset', function ( assert ) { } } ); +QUnit.test( 'translateOffsetReversed', function ( assert ) { + var tx, mapping, offset; + // Populate a transaction with bogus data + tx = new ve.dm.Transaction(); + tx.pushReplace( [], ['a','b','c'] ); + tx.pushRetain ( 5 ); + tx.pushReplace( ['d', 'e', 'f', 'g'], [] ); + tx.pushRetain( 2 ); + tx.pushStartAnnotating( 'set', { 'type': 'textStyle/bold' } ); + tx.pushRetain( 1 ); + tx.pushReplace( ['h'], ['i', 'j', 'k', 'l', 'm'] ); + tx.pushRetain( 2 ); + tx.pushReplace( [], ['n', 'o', 'p'] ); + + mapping = { + 0: 0, + 1: 0, + 2: 0, + 3: 0, + 4: 1, + 5: 2, + 6: 3, + 7: 4, + 8: 9, + 9: 10, + 10: 11, + 11: 13, + 12: 13, + 13: 13, + 14: 13, + 15: 13, + 16: 13 + }; + QUnit.expect( 17 ); + for ( offset in mapping ) { + assert.strictEqual( tx.translateOffset( Number( offset ), true ), mapping[offset] ); + } +} ); + QUnit.test( 'pushRetain', function ( assert ) { var cases = { 'retain': {