jenkins-bot has submitted this change and it was merged.
Change subject: Fix translateRange bug that expands selections
......................................................................
Fix translateRange bug that expands selections
Change-Id: Ib93e88afe8a4be61fa3306db87d6f51f6f495198
---
M src/dm/selections/ve.dm.LinearSelection.js
M src/dm/selections/ve.dm.NullSelection.js
M src/dm/selections/ve.dm.TableSelection.js
M src/dm/ve.dm.Selection.js
M src/dm/ve.dm.Surface.js
M src/dm/ve.dm.Transaction.js
M tests/dm/ve.dm.Document.test.js
M tests/dm/ve.dm.Surface.test.js
8 files changed, 145 insertions(+), 9 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/dm/selections/ve.dm.LinearSelection.js
b/src/dm/selections/ve.dm.LinearSelection.js
index 2d795b2..79e3e87 100644
--- a/src/dm/selections/ve.dm.LinearSelection.js
+++ b/src/dm/selections/ve.dm.LinearSelection.js
@@ -106,6 +106,13 @@
/**
* @inheritdoc
*/
+ve.dm.LinearSelection.prototype.translateByTransactionWithAuthor = function (
tx, author ) {
+ return new this.constructor( this.getDocument(),
tx.translateRangeWithAuthor( this.getRange(), author ) );
+};
+
+/**
+ * @inheritdoc
+ */
ve.dm.LinearSelection.prototype.getRanges = function () {
return [ this.range ];
};
@@ -130,9 +137,12 @@
* @inheritdoc
*/
ve.dm.LinearSelection.prototype.equals = function ( other ) {
- return other instanceof ve.dm.LinearSelection &&
+ return this === other || (
+ !!other &&
+ other.constructor === this.constructor &&
this.getDocument() === other.getDocument() &&
- this.getRange().equals( other.getRange() );
+ this.getRange().equals( other.getRange() )
+ );
};
/* Registration */
diff --git a/src/dm/selections/ve.dm.NullSelection.js
b/src/dm/selections/ve.dm.NullSelection.js
index a099620..9a23ecc 100644
--- a/src/dm/selections/ve.dm.NullSelection.js
+++ b/src/dm/selections/ve.dm.NullSelection.js
@@ -74,6 +74,8 @@
ve.dm.NullSelection.prototype.translateByTransaction =
ve.dm.NullSelection.prototype.clone;
+ve.dm.NullSelection.prototype.translateByTransactionWithAuthor =
ve.dm.NullSelection.prototype.clone;
+
/**
* @inheritdoc
*/
@@ -92,8 +94,11 @@
* @inheritdoc
*/
ve.dm.NullSelection.prototype.equals = function ( other ) {
- return other instanceof ve.dm.NullSelection &&
- this.getDocument() === other.getDocument();
+ return this === other || (
+ !!other &&
+ other.constructor === this.constructor &&
+ this.getDocument() === other.getDocument()
+ );
};
/**
diff --git a/src/dm/selections/ve.dm.TableSelection.js
b/src/dm/selections/ve.dm.TableSelection.js
index def74f1..f4f39ee 100644
--- a/src/dm/selections/ve.dm.TableSelection.js
+++ b/src/dm/selections/ve.dm.TableSelection.js
@@ -112,7 +112,14 @@
* @inheritdoc
*/
ve.dm.TableSelection.prototype.clone = function () {
- return new this.constructor( this.getDocument(), this.tableRange,
this.fromCol, this.fromRow, this.toCol, this.toRow );
+ return new this.constructor(
+ this.getDocument(),
+ this.tableRange,
+ this.fromCol,
+ this.fromRow,
+ this.toCol,
+ this.toRow
+ );
};
/**
@@ -303,6 +310,21 @@
};
/**
+ * @inheritdoc
+ */
+ve.dm.TableSelection.prototype.translateByTransactionWithAuthor = function (
tx, author ) {
+ var newRange = tx.translateRangeWithAuthor( this.tableRange, author );
+
+ if ( newRange.isCollapsed() ) {
+ return new ve.dm.NullSelection( this.getDocument() );
+ }
+ return new this.constructor(
+ this.getDocument(), newRange,
+ this.fromCol, this.fromRow, this.toCol, this.toRow
+ );
+};
+
+/**
* Check if the selection spans a single cell
*
* @return {boolean} The selection spans a single cell
@@ -464,13 +486,16 @@
* @inheritdoc
*/
ve.dm.TableSelection.prototype.equals = function ( other ) {
- return other instanceof ve.dm.TableSelection &&
+ return this === other || (
+ !!other &&
+ other.constructor === this.constructor &&
this.getDocument() === other.getDocument() &&
this.tableRange.equals( other.tableRange ) &&
this.fromCol === other.fromCol &&
this.fromRow === other.fromRow &&
this.toCol === other.toCol &&
- this.toRow === other.toRow;
+ this.toRow === other.toRow
+ );
};
/**
diff --git a/src/dm/ve.dm.Selection.js b/src/dm/ve.dm.Selection.js
index dc80892..e2bf78b 100644
--- a/src/dm/ve.dm.Selection.js
+++ b/src/dm/ve.dm.Selection.js
@@ -57,6 +57,11 @@
/* Methods */
/**
+ * Test for selection equality
+ */
+ve.dm.Selection.prototype.equals = null;
+
+/**
* Get a JSON serialization of this selection
*
* @abstract
@@ -140,6 +145,17 @@
ve.dm.Selection.prototype.translateByTransaction = null;
/**
+ * Apply translations from a transaction, with bias depending on author ID
comparison
+ *
+ * @abstract
+ * @method
+ * @param {ve.dm.Transaction} tx Transaction
+ * @param {number} author The selection's author ID
+ * @return {ve.dm.Selection} A new translated selection
+ */
+ve.dm.Selection.prototype.translateByTransactionWithAuthor = null;
+
+/**
* Apply translations from a set of transactions
*
* @param {ve.dm.Transaction[]} txs Transactions
diff --git a/src/dm/ve.dm.Surface.js b/src/dm/ve.dm.Surface.js
index 7e6ac5f..70ef113 100644
--- a/src/dm/ve.dm.Surface.js
+++ b/src/dm/ve.dm.Surface.js
@@ -931,7 +931,7 @@
* @fires documentUpdate
*/
ve.dm.Surface.prototype.onDocumentTransact = function ( tx ) {
- this.setSelection( this.getSelection().translateByTransaction( tx ) );
+ this.setSelection(
this.getSelection().translateByTransactionWithAuthor( tx, null ) );
this.emit( 'documentUpdate', tx );
};
diff --git a/src/dm/ve.dm.Transaction.js b/src/dm/ve.dm.Transaction.js
index 11cb1a0..0c5b42b 100644
--- a/src/dm/ve.dm.Transaction.js
+++ b/src/dm/ve.dm.Transaction.js
@@ -1109,7 +1109,7 @@
};
/**
- * Translate a range based on a transaction.
+ * Translate a range based on the transaction, with grow/shrink preference at
changes
*
* This is useful when you want to anticipate what a selection will be after a
transaction is
* processed.
@@ -1128,6 +1128,23 @@
};
/**
+ * Translate a range based on the transaction, with bias depending on author
ID comparison
+ *
+ * Biases backward if !author || !this.author || author <= this.author
+ *
+ * @see #translateOffset
+ * @param {ve.Range} range Range in the linear model before the transaction
has been processed
+ * @param {number} [author] Author ID of the range
+ * @return {ve.Range} Translated range, as it will be after processing
transaction
+ */
+ve.dm.Transaction.prototype.translateRangeWithAuthor = function ( range,
author ) {
+ var backward = !this.author || !author || author <= this.author,
+ start = this.translateOffset( range.start, backward ),
+ end = this.translateOffset( range.end, backward );
+ return range.isBackwards() ? new ve.Range( end, start ) : new ve.Range(
start, end );
+};
+
+/**
* Get the range that covers modifications made by this transaction.
*
* In the case of insertions, the range covers content the user intended to
insert.
diff --git a/tests/dm/ve.dm.Document.test.js b/tests/dm/ve.dm.Document.test.js
index e487425..c40e1ce 100644
--- a/tests/dm/ve.dm.Document.test.js
+++ b/tests/dm/ve.dm.Document.test.js
@@ -944,3 +944,54 @@
}
}
} );
+
+QUnit.test( 'Selection equality', function ( assert ) {
+ var selections, i, iLen, j, jLen, iSel, jSel, doc;
+ doc = new ve.dm.Document( [
+ { type: 'paragraph' }, 'h', 'i', { type: '/paragraph' },
+ { type: 'table' },
+ { type: 'tableSection' },
+ { type: 'tableRow' },
+ { type: 'tableCell', attributes: { colspan: 2, rowspan: 2 } },
+ { type: '/tableCell' },
+ { type: 'tableCell', attributes: {} },
+ { type: '/tableCell' },
+ { type: '/tableRow' },
+ { type: 'tableRow' },
+ { type: 'tableCell', attributes: {} },
+ { type: '/tableCell' },
+ { type: '/tableRow' },
+ { type: 'tableRow' },
+ { type: 'tableCell', attributes: {} },
+ { type: '/tableCell' },
+ { type: 'tableCell', attributes: {} },
+ { type: '/tableCell' },
+ { type: 'tableCell', attributes: {} },
+ { type: '/tableCell' },
+ { type: '/tableRow' },
+ { type: '/tableSection' },
+ { type: '/table' }
+ ] );
+ selections = [
+ new ve.dm.LinearSelection( doc, new ve.Range( 1, 1 ) ),
+ new ve.dm.LinearSelection( doc, new ve.Range( 1, 3 ) ),
+ new ve.dm.LinearSelection( doc, new ve.Range( 3, 1 ) ),
+ new ve.dm.TableSelection( doc, new ve.Range( 4, 25 ), 0, 1, 2,
2, true ),
+ new ve.dm.NullSelection( doc ),
+ undefined,
+ null,
+ 'foo'
+ ];
+ QUnit.expect( selections.length * ( selections.length - 3 ) );
+ for ( i = 0, iLen = selections.length; i < iLen; i++ ) {
+ iSel = selections[ i ];
+ if ( !( iSel instanceof ve.dm.Selection ) ) {
+ continue;
+ }
+ iSel = iSel.clone();
+ for ( j = 0, jLen = selections.length; j < jLen; j++ ) {
+ jSel = selections[ j ];
+ assert.strictEqual( iSel.equals( jSel ), i === j,
'Selections ' + i + ' and ' + j );
+ }
+ }
+} );
diff --git a/tests/dm/ve.dm.Surface.test.js b/tests/dm/ve.dm.Surface.test.js
index 48cd63f..5a4ad57 100644
--- a/tests/dm/ve.dm.Surface.test.js
+++ b/tests/dm/ve.dm.Surface.test.js
@@ -95,6 +95,18 @@
} );
+QUnit.test( 'range translation', 2, function ( assert ) {
+ var sel, range,
+ surface = new ve.dm.SurfaceStub( null, new ve.Range( 3 ) ),
+ doc = surface.getDocument(),
+ tx = ve.dm.Transaction.newFromInsertion( doc, 3, [ 'x' ] );
+ surface.change( tx );
+ sel = surface.getSelection();
+ assert.ok( sel instanceof ve.dm.LinearSelection, 'Selection is linear'
);
+ range = sel.getRange();
+ assert.deepEqual( { from: range.from, to: range.to }, { from: 3, to: 3
}, 'Cursor is unmoved' );
+} );
+
QUnit.test( 'staging', 37, function ( assert ) {
var tx1, tx2,
surface = new ve.dm.SurfaceStub( null, new ve.Range( 1, 3 ) ),
--
To view, visit https://gerrit.wikimedia.org/r/317330
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib93e88afe8a4be61fa3306db87d6f51f6f495198
Gerrit-PatchSet: 10
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
