jenkins-bot has submitted this change and it was merged.
Change subject: Introduce newFromDocumentReplace() transaction builder
......................................................................
Introduce newFromDocumentReplace() transaction builder
Replaces newFromNodeReplacement(). newFromNodeReplacement was very
simplistic and didn't support metadata or internal list items, so
if you had comments or references inside of the data you were editing
(reference contents or an image caption), they'd get mangled.
With this, you can do:
newDoc = doc.getDocumentSlice( node );
// Edit newDoc
tx = ve.dm.Transaction.newFromDocumentReplace( doc, node, newDoc );
surface.change( newDoc );
and that takes care of metadata, internal list items, and things like
references that reference internal list items.
ve.dm.Document.js:
* In getDocumentSlice(), store a reference to the original document
and the number of items in its InternalList at the time of slicing
in the created slice. This is used for reconciliation when the
modified slice is injected back into the parent document with
newFromDocumentReplace().
ve.dm.InternalList.js:
* Add a method for merging in another InternalList. This provides a
mapping from old to new InternalList indexes so the linear model data
being injected by newFromDocumentReplace() can have its InternalList
indexes remapped.
ve.dm.Transaction.js:
* Replace newFromNodeReplacement() with newFromDocumentReplace()
ve.ui.MWMediaEditDialog.js, ve.ui.MWReferenceDialog.js:
* Use getDocumentSlice/newFromDocumentReplace for editing captions/refs
* Change insertion code path to insert an empty internalItem/caption, then
newFromDocumentReplace into that
* Add empty internalList to new mini-documents
ve/test/dm/ve.dm.Transaction.test.js:
* Replace newFromNodeReplacement tests with newFromDocumentReplace tests
ve-mw/test/dm/ve.dm.Transaction.test.js (new):
* Add tests for newFromDocumentReplace with mwReference nodes
ve.dm.mwExample.js:
* Add data for newFromDocumentReplace with mwReference tests
VisualEditor.hooks.php:
* Add new test file
Bug: 52102
Change-Id: I4aa980780114b391924f04df588e81c990c32983
---
M VisualEditor.hooks.php
A modules/ve-mw/test/dm/ve.dm.Transaction.test.js
M modules/ve-mw/test/dm/ve.dm.mwExample.js
M modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js
M modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
M modules/ve/dm/ve.dm.Document.js
M modules/ve/dm/ve.dm.InternalList.js
M modules/ve/dm/ve.dm.Transaction.js
M modules/ve/test/dm/ve.dm.Transaction.test.js
9 files changed, 550 insertions(+), 68 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/VisualEditor.hooks.php b/VisualEditor.hooks.php
index d505def..0e87f07 100644
--- a/VisualEditor.hooks.php
+++ b/VisualEditor.hooks.php
@@ -341,6 +341,7 @@
've/test/dm/ve.dm.InternalList.test.js',
've-mw/test/dm/ve.dm.InternalList.test.js',
've/test/dm/ve.dm.Transaction.test.js',
+ 've-mw/test/dm/ve.dm.Transaction.test.js',
've/test/dm/ve.dm.TransactionProcessor.test.js',
've/test/dm/ve.dm.Surface.test.js',
've/test/dm/ve.dm.SurfaceFragment.test.js',
diff --git a/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
new file mode 100644
index 0000000..c38b7c1
--- /dev/null
+++ b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js
@@ -0,0 +1,145 @@
+/*!
+ * VisualEditor DataModel Transaction tests.
+ *
+ * @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt
+ * @license The MIT License (MIT); see LICENSE.txt
+ */
+
+QUnit.module( 've.dm.Transaction' );
+
+// FIXME duplicates test runner; should be using a data provider
+QUnit.test( 'newFromDocumentReplace with references', function ( assert ) {
+ var i, j, doc2, tx, actualStoreItems, expectedStoreItems,
+ doc = ve.dm.example.createExampleDocument( 'internalData' ),
+ complexDoc = ve.dm.mwExample.createExampleDocument(
'complexInternalData' ),
+ comment = { 'type': 'alienMeta', 'attributes': { 'domElements':
$( '<!-- hello -->' ).get() } },
+ withReference = [
+ { 'type': 'paragraph' },
+ 'B', 'a', 'r',
+ { 'type': 'mwReference', 'attributes': {
+ 'mw': {},
+ 'about': '#mwt4',
+ 'listIndex': 0,
+ 'listGroup': 'mwReference/',
+ 'listKey': null,
+ 'refGroup': '',
+ 'contentsUsed': true
+ } },
+ { 'type': '/mwReference' },
+ { 'type': '/paragraph' },
+ { 'type': 'internalList' },
+ { 'type': 'internalItem' },
+ { 'type': 'paragraph', 'internal': { 'generated':
'wrapper' } },
+ 'B',
+ 'a',
+ 'z',
+ { 'type': '/paragraph' },
+ { 'type': '/internalItem' },
+ { 'type': '/internalList' }
+ ],
+ cases = [
+ {
+ 'msg': 'metadata insertion',
+ 'doc': complexDoc,
+ 'range': new ve.Range( 0, 7 ),
+ 'modify': function ( newDoc ) {
+ newDoc.commit(
ve.dm.Transaction.newFromMetadataInsertion(
+ newDoc, 5, 0, [ comment ]
+ ) );
+ },
+ 'expectedOps': [
+ {
+ 'type': 'replace',
+ 'remove': complexDoc.getData(
new ve.Range( 0, 7 ) ),
+ 'insert': complexDoc.getData(
new ve.Range( 0, 7 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 0, 7 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 0, 5 ) )
+ .concat( [ [ comment ]
] )
+ .concat(
complexDoc.getMetadata( new ve.Range( 6, 8 ) ) )
+ },
+ { 'type': 'retain', 'length': 1 },
+ {
+ 'type': 'replace',
+ 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
+ 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
+ },
+ { 'type': 'retain', 'length': 1 }
+ ]
+ },
+ {
+ 'msg': 'metadata removal',
+ 'doc': complexDoc,
+ 'range': new ve.Range( 24, 31 ),
+ 'modify': function ( newDoc ) {
+ newDoc.commit(
ve.dm.Transaction.newFromMetadataRemoval(
+ newDoc, 6, new ve.Range( 0, 1 )
+ ) );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 8 },
+ {
+ 'type': 'replace',
+ 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
+ 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 30 ) )
+ .concat( [ [] ] )
+ .concat(
complexDoc.getMetadata( new ve.Range( 31, 32 ) ) )
+ },
+ { 'type': 'retain', 'length': 1 }
+ ]
+ },
+ {
+ 'msg': 'inserting a brand new document;
internal lists are merged and items renumbered',
+ 'doc': complexDoc,
+ 'range': new ve.Range( 7, 7 ),
+ 'newDocData': withReference,
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 7 },
+ {
+ 'type': 'replace',
+ 'remove': [],
+ 'insert': withReference.slice(
0, 4 )
+ // Renumber listIndex
from 0 to 2
+ .concat( [
ve.extendObject( true, {}, withReference[4],
+ { 'attributes':
{ 'listIndex': 2 } } ) ] )
+ .concat(
withReference.slice( 5, 7 ) )
+ },
+ { 'type': 'retain', 'length': 1 },
+ {
+ 'type': 'replace',
+ 'remove': complexDoc.getData(
new ve.Range( 8, 32 ) ),
+ 'insert': complexDoc.getData(
new ve.Range( 8, 32 ) )
+ .concat(
withReference.slice( 8, 15 ) ),
+ 'removeMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) ),
+ 'insertMetadata':
complexDoc.getMetadata( new ve.Range( 8, 32 ) )
+ .concat( new Array( 7 )
)
+ },
+ { 'type': 'retain', 'length': 1 }
+ ]
+ }
+ ];
+ QUnit.expect( 2 * cases.length );
+ for ( i = 0; i < cases.length; i++ ) {
+ doc = cases[i].doc; // TODO deep copy?
+ if ( cases[i].newDocData ) {
+ doc2 = new ve.dm.Document( cases[i].newDocData
);
+ } else {
+ doc2 = doc.getDocumentSlice( cases[i].range );
+ cases[i].modify( doc2 );
+ }
+ tx = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
+ assert.deepEqualWithDomElements( tx.getOperations(),
cases[i].expectedOps, cases[i].msg + ': transaction' );
+
+ actualStoreItems = [];
+ expectedStoreItems = cases[i].expectedStoreItems || [];
+ for ( j = 0; j < expectedStoreItems.length; j++ ) {
+ actualStoreItems[j] = doc.store.value(
doc.store.indexOfHash(
+ ve.getHash( expectedStoreItems[j] )
+ ) );
+ }
+ assert.deepEqual( actualStoreItems, expectedStoreItems,
cases[i].msg + ': store items' );
+ }
+} );
\ No newline at end of file
diff --git a/modules/ve-mw/test/dm/ve.dm.mwExample.js
b/modules/ve-mw/test/dm/ve.dm.mwExample.js
index adb0570..bd663ef 100644
--- a/modules/ve-mw/test/dm/ve.dm.mwExample.js
+++ b/modules/ve-mw/test/dm/ve.dm.mwExample.js
@@ -570,6 +570,67 @@
{ 'type': '/internalList' }
];
+ve.dm.mwExample.complexInternalData = [
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!-- before
-->' ).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': 'paragraph' },
+ 'F', ['o', [ve.dm.example.bold]], ['o', [ve.dm.example.italic]],
+ { 'type': 'mwReference', 'attributes': {
+ 'mw': {},
+ 'about': '#mwt1',
+ 'listIndex': 0,
+ 'listGroup': 'mwReference/',
+ 'listKey': null,
+ 'refGroup': '',
+ 'contentsUsed': true
+ } },
+ { 'type': '/mwReference' },
+ { 'type': '/paragraph' },
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!-- after
-->' ).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': 'internalList' },
+ { 'type': 'internalItem' },
+ { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
+ 'R', ['e', [ve.dm.example.bold]], 'f',
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!--
reference -->' ).get() } },
+ { 'type': '/alienMeta' },
+ 'e', ['r', [ve.dm.example.italic]], ['e', [ve.dm.example.italic]],
+ { 'type': 'mwReference', 'attributes': {
+ 'mw': {},
+ 'about': '#mwt2',
+ 'listIndex': 1,
+ 'listGroup': 'mwReference/',
+ 'listKey': 'foo',
+ 'refGroup': '',
+ 'contentsUsed': true
+ } },
+ { 'type': '/mwReference' },
+ 'n', 'c', 'e',
+ { 'type': '/paragraph' },
+ { 'type': '/internalItem' },
+ { 'type': 'internalItem' },
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!--
beginning -->' ).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': 'preformatted' },
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!-- inside
-->' ).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': 'mwEntity', 'attributes': { 'character': '€' } },
+ { 'type': '/mwEntity' },
+ '2', '5', '0',
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!-- inside2
-->' ).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': '/preformatted' },
+ { 'type': 'alienMeta', 'attributes': { 'domElements': $( '<!-- end -->'
).get() } },
+ { 'type': '/alienMeta' },
+ { 'type': '/internalItem' },
+ { 'type': '/internalList' }
+];
+
+ve.dm.mwExample.complexInternalData.internalItems = [
+ { 'group': 'mwReference', 'key': null, 'body': 'First reference' },
+ { 'group': 'mwReference', 'key': 'foo', 'body': 'Table in ref:
<table><tr><td>because I can</td></tr></table>' }
+];
+
ve.dm.mwExample.domToDataCases = {
'adjacent annotations': {
'html':
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js
index 6467a55..6bfd55e 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js
@@ -20,6 +20,7 @@
ve.ui.MWDialog.call( this, surface, config );
// Properties
+ this.mediaNode = null;
this.captionNode = null;
};
@@ -87,24 +88,27 @@
/** */
ve.ui.MWMediaEditDialog.prototype.onOpen = function () {
- var data, doc = this.surface.getModel().getDocument();
+ var newDoc, doc = this.surface.getModel().getDocument();
// Parent method
ve.ui.MWDialog.prototype.onOpen.call( this );
// Properties
- this.captionNode =
this.surface.getView().getFocusedNode().getModel().getCaptionNode();
+ this.mediaNode = this.surface.getView().getFocusedNode().getModel();
+ this.captionNode = this.mediaNode.getCaptionNode();
if ( this.captionNode && this.captionNode.getLength() > 0 ) {
- data = doc.getData( this.captionNode.getRange(), true );
+ newDoc = doc.getDocumentSlice( this.captionNode );
} else {
- data = [
+ newDoc = [
{ 'type': 'paragraph', 'internal': { 'generated':
'wrapper' } },
- { 'type': '/paragraph' }
+ { 'type': '/paragraph' },
+ { 'type': 'internalList' },
+ { 'type': '/internalList' }
];
}
this.captionSurface = new ve.ui.SurfaceWidget(
- new ve.dm.ElementLinearData( doc.getStore(), data ),
+ newDoc,
{
'$$': this.frame.$$,
'tools': this.constructor.static.toolbarGroups,
@@ -119,28 +123,26 @@
/** */
ve.ui.MWMediaEditDialog.prototype.onClose = function ( action ) {
- var data, doc, surfaceModel = this.surface.getModel();
+ var newDoc, doc, surfaceModel = this.surface.getModel();
// Parent method
ve.ui.MWDialog.prototype.onClose.call( this );
if ( action === 'apply' ) {
- data = this.captionSurface.getContent();
+ newDoc =
this.captionSurface.getSurface().getModel().getDocument();
doc = surfaceModel.getDocument();
- if ( this.captionNode ) {
- // Replace the contents of the caption
- surfaceModel.getFragment( this.captionNode.getRange(),
true ).insertContent( data );
- } else {
+ if ( !this.captionNode ) {
// Insert a new caption at the beginning of the image
node
surfaceModel.getFragment()
.adjustRange( 1 )
.collapseRangeToStart()
- .insertContent(
- [ { 'type': 'mwImageCaption' } ]
- .concat( data )
- .concat( [ { 'type':
'/mwImageCaption' } ] )
- );
+ .insertContent( [ { 'type': 'mwImageCaption' },
{ 'type': '/mwImageCaption' } ] );
+ this.captionNode = this.mediaNode.getCaptionNode();
}
+ // Replace the contents of the caption
+ surfaceModel.change(
+ ve.dm.Transaction.newFromDocumentReplace( doc,
this.captionNode, newDoc )
+ );
}
// Cleanup
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
index 7030907..0405683 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
@@ -170,7 +170,7 @@
* @inheritdoc
*/
ve.ui.MWReferenceDialog.prototype.onClose = function ( action ) {
- var i, len, txs, item, data, group, refGroup, listGroup, keyIndex,
refNodes,
+ var i, len, txs, item, newDoc, group, refGroup, listGroup, keyIndex,
refNodes,
surfaceModel = this.surface.getModel(),
// Store the original selection browsers may reset it after
// the first model change.
@@ -182,7 +182,7 @@
ve.ui.MWDialog.prototype.onClose.call( this, action );
if ( action === 'insert' || action === 'apply' ) {
- data = this.referenceSurface.getContent();
+ newDoc =
this.referenceSurface.getSurface().getModel().getDocument();
refGroup = this.referenceGroupInput.getValue();
listGroup = 'mwReference/' + refGroup;
@@ -223,8 +223,8 @@
}
// Update internal node content
surfaceModel.change(
- ve.dm.Transaction.newFromNodeReplacement(
- doc, internalList.getItemNode(
this.ref.listIndex ), data
+ ve.dm.Transaction.newFromDocumentReplace(
+ doc, internalList.getItemNode(
this.ref.listIndex ), newDoc
)
);
}
@@ -239,9 +239,15 @@
'listGroup': listGroup,
'refGroup': refGroup
};
- item = internalList.getItemInsertion(
this.ref.listGroup, this.ref.listKey, data );
+ // Insert an internal item, then inject the
subdocument into it
+ item = internalList.getItemInsertion(
this.ref.listGroup, this.ref.listKey, [] );
surfaceModel.change( item.transaction );
this.ref.listIndex = item.index;
+ surfaceModel.change(
+
ve.dm.Transaction.newFromDocumentReplace(
+ doc, internalList.getItemNode(
this.ref.listIndex ), newDoc
+ )
+ );
}
// Add reference at cursor
surfaceModel.getFragment( selection
).collapseRangeToEnd().insertContent( [
@@ -262,7 +268,7 @@
* @chainable
*/
ve.ui.MWReferenceDialog.prototype.useReference = function ( ref ) {
- var data, refGroup,
+ var newDoc, refGroup,
doc = this.surface.getModel().getDocument();
if ( ref ) {
@@ -273,15 +279,17 @@
'refGroup': ref.refGroup,
'listIndex': ref.listIndex
};
- data = doc.getData( doc.getInternalList().getItemNode(
ref.listIndex ).getRange(), true );
+ newDoc = doc.getDocumentSlice(
doc.getInternalList().getItemNode( ref.listIndex ) );
refGroup = ref.refGroup;
} else {
// Create a new reference
this.ref = null;
- data = [
+ newDoc = new ve.dm.Document( [
{ 'type': 'paragraph', 'internal': { 'generated':
'wrapper' } },
- { 'type': '/paragraph' }
- ];
+ { 'type': '/paragraph' },
+ { 'type': 'internalList' },
+ { 'type': '/internalList' }
+ ] );
refGroup = '';
}
@@ -292,7 +300,7 @@
// Properties
this.referenceSurface = new ve.ui.SurfaceWidget(
- new ve.dm.ElementLinearData( doc.getStore(), data ),
+ newDoc,
{
'$$': this.frame.$$,
'tools': this.constructor.static.toolbarGroups,
diff --git a/modules/ve/dm/ve.dm.Document.js b/modules/ve/dm/ve.dm.Document.js
index daa30ac..d71e9c1 100644
--- a/modules/ve/dm/ve.dm.Document.js
+++ b/modules/ve/dm/ve.dm.Document.js
@@ -312,7 +312,7 @@
* @throws {Error} rangeOrNode must be a ve.Range or a ve.dm.Node
*/
ve.dm.Document.prototype.getDocumentSlice = function ( rangeOrNode ) {
- var data, range,
+ var data, range, newDoc,
store = this.store.clone(),
listRange = this.internalList.getListNode().getOuterRange();
if ( rangeOrNode instanceof ve.dm.Node ) {
@@ -327,10 +327,15 @@
// The range does not include the entire internal list, so add
it
data = data.concat( this.getFullData( listRange ) );
}
- return new this.constructor(
+ newDoc = new this.constructor(
new ve.dm.ElementLinearData( store, data ),
undefined, this.internalList
);
+ // Record the length of the internal list at the time the slice was
created so we can
+ // reconcile additions properly
+ newDoc.origDoc = this;
+ newDoc.origInternalListLength = this.internalList.getItemNodeCount();
+ return newDoc;
};
/**
diff --git a/modules/ve/dm/ve.dm.InternalList.js
b/modules/ve/dm/ve.dm.InternalList.js
index e69a969..05966d8 100644
--- a/modules/ve/dm/ve.dm.InternalList.js
+++ b/modules/ve/dm/ve.dm.InternalList.js
@@ -410,3 +410,56 @@
ve.dm.InternalList.prototype.clone = function ( doc ) {
return new this.constructor( doc || this.getDocument() );
};
+
+/**
+ * Merge another internal list into this one.
+ *
+ * This function updates the state of this list, and returns a mapping from
indexes in list to
+ * indexes in this, as well as a set of ranges that should be copied from
list's linear model
+ * into this list's linear model by the caller.
+ *
+ * @param {ve.dm.InternalList} list Internal list to merge into this list
+ * @param {number} commonLength The number of elements, counted from the
beginning, that the lists have in common
+ * @returns {Object} 'mapping' is an object mapping indexes in list to indexes
in this; newItemRanges is an array
+ * of ranges of internal nodes in list's document that should be copied into
our document
+ */
+ve.dm.InternalList.prototype.merge = function ( list, commonLength ) {
+ var i, k, key,
+ listLen = list.getItemNodeCount(),
+ nextIndex = this.getItemNodeCount(),
+ newItemRanges = [],
+ mapping = {};
+ for ( i = 0; i < commonLength; i++ ) {
+ mapping[i] = i;
+ }
+ for ( i = commonLength; i < listLen; i++ ) {
+ // Try to find i in list.keyIndexes
+ key = undefined;
+ for ( k in list.keyIndexes ) {
+ if ( list.keyIndexes[k] === i ) {
+ key = k;
+ break;
+ }
+ }
+
+ if ( this.keyIndexes[key] !== undefined ) {
+ // We already have this key in this internal list.
Ignore the duplicate that the other
+ // list is trying to merge in.
+ // NOTE: This case cannot occur in VE currently, but
may be possible in the future with
+ // collaborative editing, which is why this code needs
to be rewritten before we do
+ // collaborative editing.
+ mapping[i] = this.keyIndexes[key];
+ } else {
+ mapping[i] = nextIndex;
+ if ( key !== undefined ) {
+ this.keyIndexes[key] = nextIndex;
+ }
+ nextIndex++;
+ newItemRanges.push( list.getItemNode( i
).getOuterRange() );
+ }
+ }
+ return {
+ 'mapping': mapping,
+ 'newItemRanges': newItemRanges
+ };
+};
diff --git a/modules/ve/dm/ve.dm.Transaction.js
b/modules/ve/dm/ve.dm.Transaction.js
index cfbc85e..a94f188 100644
--- a/modules/ve/dm/ve.dm.Transaction.js
+++ b/modules/ve/dm/ve.dm.Transaction.js
@@ -152,27 +152,122 @@
};
/**
- * Generate a transaction that replaces the contents of a branch node.
+ * Build a transaction that replaces the contents of a node with the contents
of a document.
*
- * The node whose contents are being replaced should be an unrestricted branch
node.
+ * This is typically used to merge changes to a document slice back into the
main document. If newDoc
+ * is a document slice of doc, it's assumed that there were no changes to
doc's internal list since
+ * the slice, so any differences between internal items that doc and newDoc
have in common will
+ * be resolved in newDoc's favor.
*
- * @param {ve.dm.Document} doc Document to create transaction for
- * @param {ve.dm.Node|ve.Range} nodeOrRange Branch node or inner range of such
- * @param {Array} newData Linear model data to replace the contents of the
node with
- * @returns {ve.dm.Transaction} Transaction that replaces the contents of the
node
- * @throws {Error} nodeOrRange must be a ve.dm.Node or a ve.Range
+ * @param {ve.dm.Document} doc Main document
+ * @param {ve.Range|ve.dm.Node} removeNodeOrRange Node or range to remove
+ * @param {ve.dm.Document} newDoc Document to insert
+ * @returns {ve.dm.Transaction} Transaction that replaces the node and updates
the internal list
+ * @throws {Error} removeNodeOrRange must be a ve.dm.Node or a ve.Range
*/
-ve.dm.Transaction.newFromNodeReplacement = function ( doc, nodeOrRange,
newData ) {
- var tx = new ve.dm.Transaction(), range = nodeOrRange;
- if ( range instanceof ve.dm.Node ) {
- range = range.getRange();
+ve.dm.Transaction.newFromDocumentReplace = function ( doc, removeNodeOrRange,
newDoc ) {
+ var i, len, range, merge, data, metadata, listData, listMetadata,
oldEndOffset, newEndOffset,
+ listNode = doc.internalList.getListNode(),
+ listNodeRange = listNode.getRange(),
+ newListNode = newDoc.internalList.getListNode(),
+ newListNodeRange = newListNode.getRange(),
+ newListNodeOuterRange = newListNode.getOuterRange(),
+ tx = new ve.dm.Transaction();
+
+ if ( removeNodeOrRange instanceof ve.dm.Node ) {
+ range = removeNodeOrRange.getRange();
+ } else if ( removeNodeOrRange instanceof ve.Range ) {
+ range = removeNodeOrRange;
+ } else {
+ throw new Error( 'removeNodeOrRange must be a ve.dm.Node or a
ve.Range' );
}
- if ( !( range instanceof ve.Range ) ) {
- throw new Error( 'nodeOrRange must be a ve.dm.Node or a
ve.Range' );
+
+ // Get the data and the metadata, but skip over the internal list
+ data = new ve.dm.ElementLinearData( doc.getStore(),
+ newDoc.getData( new ve.Range( 0, newListNodeOuterRange.start ),
true ).concat(
+ newDoc.getData( new ve.Range(
newListNodeOuterRange.end, newDoc.data.getLength() ), true )
+ )
+ );
+ metadata = new ve.dm.MetaLinearData( doc.getStore(),
+ newDoc.getMetadata( new ve.Range( 0,
newListNodeOuterRange.start ), true ).concat(
+ // Merge the metadata immediately before and
immediately after the internal list
+ ve.copy( ve.dm.MetaLinearData.static.merge( [
+ newDoc.metadata.getData(
newListNodeOuterRange.start ),
+ newDoc.metadata.getData(
newListNodeOuterRange.end )
+ ] ) )
+ ).concat( newDoc.getMetadata(
+ new ve.Range( newListNodeOuterRange.end,
newDoc.data.getLength() ), true
+ ) )
+ );
+ // Merge the stores
+ merge = doc.getStore().merge( newDoc.getStore() );
+ // Remap the store indexes in the data
+ data.remapStoreIndexes( merge );
+
+ merge = doc.internalList.merge( newDoc.internalList,
newDoc.origInternalListLength || 0 );
+ // Remap the indexes in the data
+ data.remapInteralListIndexes( merge.mapping );
+ // Get data for the new internal list
+ if ( newDoc.origDoc === doc ) {
+ // newDoc is a document slice based on doc, so all the internal
list items present in doc
+ // when it was cloned are also in newDoc. We need to get the
newDoc version of these items
+ // so that changes made in newDoc are reflected.
+ if ( newDoc.origInternalListLength > 0 ) {
+ oldEndOffset = doc.internalList.getItemNode(
newDoc.origInternalListLength - 1 ).getOuterRange().end;
+ newEndOffset = newDoc.internalList.getItemNode(
newDoc.origInternalListLength - 1 ).getOuterRange().end;
+ } else {
+ oldEndOffset = listNodeRange.start;
+ newEndOffset = newListNodeRange.start;
+ }
+ listData = newDoc.getData( new ve.Range(
newListNodeRange.start, newEndOffset ), true )
+ .concat( doc.getData( new ve.Range( oldEndOffset,
listNodeRange.end ), true ) );
+ listMetadata = newDoc.getMetadata( new ve.Range(
newListNodeRange.start, newEndOffset ), true )
+ .concat( doc.getMetadata( new ve.Range( oldEndOffset,
listNodeRange.end ) , true ) );
+ } else {
+ // newDoc is brand new, so use doc's internal list as a base
+ listData = doc.getData( listNodeRange, true );
+ listMetadata = doc.getMetadata( listNodeRange, true );
}
- tx.pushRetain( range.start );
- tx.pushReplace( doc, range.start, range.end - range.start, newData );
- tx.pushFinalRetain( doc, range.end );
+ for ( i = 0, len = merge.newItemRanges.length; i < len; i++ ) {
+ listData = listData.concat( newDoc.getData(
merge.newItemRanges[i], true ) );
+ // We don't have to worry about merging metadata at the edges,
because there can't be
+ // metadata between internal list items
+ listMetadata = listMetadata.concat( newDoc.getMetadata(
merge.newItemRanges[i], true ) );
+ }
+
+ if ( range.end <= listNodeRange.start ) {
+ // range is entirely before listNodeRange
+ // First replace the node, then the internal list
+ tx.pushRetain( range.start );
+ tx.pushReplace( doc, range.start, range.end - range.start,
data.data, metadata.data );
+ tx.pushRetain( listNodeRange.start - range.end );
+ tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
+ listData, listMetadata
+ );
+ tx.pushRetain( doc.data.getLength() - listNodeRange.end );
+ } else if ( listNodeRange.end <= range.start ) {
+ // range is entirely after listNodeRange
+ // First replace the internal list, then the node
+ tx.pushRetain( listNodeRange.start );
+ tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
+ listData, listMetadata
+ );
+ tx.pushRetain( range.start - listNodeRange.end );
+ tx.pushReplace( doc, range.start, range.end - range.start,
data.data, metadata.data );
+ tx.pushRetain( doc.data.getLength() - range.end );
+ } else if ( range.start >= listNodeRange.start && range.end <=
listNodeRange.end ) {
+ // range is entirely within listNodeRange
+ // Merge data into listData, then only replace the internal list
+ ve.batchSplice( listData, range.start - listNodeRange.start,
+ range.end - range.start, data.data );
+ ve.batchSplice( listMetadata, range.start - listNodeRange.start,
+ range.end - range.start + 1, metadata.data );
+ tx.pushRetain( listNodeRange.start );
+ tx.pushReplace( doc, listNodeRange.start, listNodeRange.end -
listNodeRange.start,
+ listData, listMetadata
+ );
+ tx.pushRetain( doc.data.getLength() - listNodeRange.end );
+ }
return tx;
};
diff --git a/modules/ve/test/dm/ve.dm.Transaction.test.js
b/modules/ve/test/dm/ve.dm.Transaction.test.js
index 9e3851f..5e09cf2 100644
--- a/modules/ve/test/dm/ve.dm.Transaction.test.js
+++ b/modules/ve/test/dm/ve.dm.Transaction.test.js
@@ -668,38 +668,150 @@
runConstructorTests( assert, ve.dm.Transaction.newFromRemoval, cases );
} );
-QUnit.test( 'newFromNodeReplacement', function ( assert ) {
- var doc = ve.dm.example.createExampleDocument( 'internalData' ),
- paragraph = [ { 'type': 'paragraph' }, 'H', 'e', 'l', 'l', 'o',
{ 'type': '/paragraph' } ],
- secondNode = doc.internalList.getItemNode( 1 ),
- cases = {
- 'replacing first internal node with paragraph': {
- 'args': [doc, new ve.Range( 7, 12 ), paragraph],
- 'ops': [
- { 'type': 'retain', 'length': 7 },
+QUnit.test( 'newFromDocumentReplace', function ( assert ) {
+ var i, j, doc2, tx, actualStoreItems, expectedStoreItems,
+ doc = ve.dm.example.createExampleDocument( 'internalData' ),
+ nextIndex = doc.store.valueStore.length,
+ whee = [ { 'type': 'paragraph' }, 'W', 'h', 'e', 'e', { 'type':
'/paragraph' } ],
+ wheeItem = [ { 'type': 'internalItem' } ].concat( whee
).concat( [ { 'type': '/internalItem' } ] ),
+ cases = [
+ {
+ 'msg': 'simple insertion',
+ 'doc': 'internalData',
+ 'range': new ve.Range( 7, 12 ),
+ 'modify': function ( newDoc ) {
+ // Change "Bar" to "Bazaar"
+ newDoc.commit(
ve.dm.Transaction.newFromInsertion(
+ newDoc, 3, [ 'z', 'a', 'a' ]
+ ) );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 6 },
{
'type': 'replace',
- 'remove': doc.data.slice( 7, 12
),
- 'insert': paragraph
+ 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
+ 'insert': doc.getData( new
ve.Range( 6, 10 ) )
+ .concat( [ 'z', 'a',
'a' ] )
+ .concat( doc.getData(
new ve.Range( 10, 20 ) ) )
},
- { 'type': 'retain', 'length': 15 }
+ { 'type': 'retain', 'length': 7 }
]
},
- 'replacing second internal node with two paragraphs': {
- 'args': [doc, secondNode, paragraph.concat(
paragraph )],
- 'ops': [
- { 'type': 'retain', 'length': 14 },
+ {
+ 'msg': 'simple annotation',
+ 'doc': 'internalData',
+ 'range': doc.getInternalList().getItemNode( 1 ),
+ 'modify': function ( newDoc ) {
+ // Bold the first two characters
+ newDoc.commit(
ve.dm.Transaction.newFromAnnotation(
+ newDoc, new ve.Range( 1, 3 ),
'set', ve.dm.example.bold
+ ) );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 6 },
{
'type': 'replace',
- 'remove':
doc.data.getDataSlice( secondNode.getRange() ),
- 'insert': paragraph.concat(
paragraph )
+ 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
+ 'insert': doc.getData( new
ve.Range( 6, 15 ) )
+ .concat( [ [
doc.data.getData( 15 ), [ nextIndex ] ] ] )
+ .concat( [ [
doc.data.getData( 16 ), [ nextIndex ] ] ] )
+ .concat( doc.getData(
new ve.Range( 17, 20 ) ) )
},
- { 'type': 'retain', 'length': 8 }
+ { 'type': 'retain', 'length': 7 }
+ ],
+ 'expectedStoreItems': [
+ ve.dm.example.bold
+ ]
+ },
+ {
+ 'msg': 'insertion into internal list',
+ 'doc': 'internalData',
+ 'range': new ve.Range( 21, 27 ),
+ 'modify': function ( newDoc ) {
+ var insertion =
newDoc.internalList.getItemInsertion( 'test', 'whee', whee );
+ newDoc.commit( insertion.transaction );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 6 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
+ 'insert': doc.getData( new
ve.Range( 6, 20 ) ).concat( wheeItem )
+ },
+ { 'type': 'retain', 'length': 1 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData( new
ve.Range( 21, 27 ) ),
+ 'insert': doc.getData( new
ve.Range( 21, 27 ) )
+ }
+ ]
+ },
+ {
+ 'msg': 'change in internal list',
+ 'doc': 'internalData',
+ 'range': new ve.Range( 21, 27 ),
+ 'modify': function ( newDoc ) {
+ newDoc.commit(
ve.dm.Transaction.newFromInsertion(
+ newDoc, 12, [ '!', '!', '!' ]
+ ) );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 6 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
+ 'insert': doc.getData( new
ve.Range( 6, 11 ) )
+ .concat( [ '!', '!',
'!' ] )
+ .concat( doc.getData(
new ve.Range( 11, 20 ) ) )
+ },
+ { 'type': 'retain', 'length': 1 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData( new
ve.Range( 21, 27 ) ),
+ 'insert': doc.getData( new
ve.Range( 21, 27 ) )
+ }
+ ]
+ },
+ {
+ 'msg': 'insertion into internal list from slice
within internal list',
+ 'doc': 'internalData',
+ 'range': new ve.Range( 7, 12 ),
+ 'modify': function ( newDoc ) {
+ var insertion =
newDoc.internalList.getItemInsertion( 'test', 'whee', whee );
+ newDoc.commit( insertion.transaction );
+ },
+ 'expectedOps': [
+ { 'type': 'retain', 'length': 6 },
+ {
+ 'type': 'replace',
+ 'remove': doc.getData( new
ve.Range( 6, 20 ) ),
+ 'insert': doc.getData( new
ve.Range( 6, 20 ) ).concat( wheeItem )
+ },
+ { 'type': 'retain', 'length': 7 }
]
}
- };
- QUnit.expect( ve.getObjectKeys( cases ).length );
- runConstructorTests( assert, ve.dm.Transaction.newFromNodeReplacement,
cases );
+ ];
+ QUnit.expect( 2 * cases.length );
+ for ( i = 0; i < cases.length; i++ ) {
+ doc = ve.dm.example.createExampleDocument( cases[i].doc );
+ if ( cases[i].newDocData ) {
+ doc2 = new ve.dm.Document( cases[i].newDocData );
+ } else {
+ doc2 = doc.getDocumentSlice( cases[i].range );
+ cases[i].modify( doc2 );
+ }
+ tx = ve.dm.Transaction.newFromDocumentReplace( doc,
cases[i].range, doc2 );
+ assert.deepEqualWithDomElements( tx.getOperations(),
cases[i].expectedOps, cases[i].msg + ': transaction' );
+
+ actualStoreItems = [];
+ expectedStoreItems = cases[i].expectedStoreItems || [];
+ for ( j = 0; j < expectedStoreItems.length; j++ ) {
+ actualStoreItems[j] = doc.store.value(
doc.store.indexOfHash(
+ ve.getHash( expectedStoreItems[j] )
+ ) );
+ }
+ assert.deepEqual( actualStoreItems, expectedStoreItems,
cases[i].msg + ': store items' );
+ }
} );
QUnit.test( 'newFromAttributeChanges', function ( assert ) {
var doc = ve.dm.example.createExampleDocument(),
--
To view, visit https://gerrit.wikimedia.org/r/82790
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4aa980780114b391924f04df588e81c990c32983
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
