wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-09-25 19:26:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Correctly preserve metadata in Transaction.newFromUnwrap.

Browse source 
The `Transaction.pushReplace` method has a corner case if the removed
region has metadata and the inserted region is empty.  This works fine
unless there are two adjacent `pushReplace` operations, which can occur
in `Transaction.newFromUnwrap`.  Fix this by having `pushReplace` look
at a preceding replace and correctly merge the two operations if
possible (in particular in the tricky case where the previous case has
a zero-length insertion).  Pleasantly, this can be done without a lot of
special-casing code in `pushReplace` or `newFromUnwrap`.

Add test cases verifying the `newFromUnwrap` works correctly (both
in commit and in rollback) when there is metadata present.

Change-Id: I6cfec0d2b1823dad724422f018a3c73dc0c7f186
This commit is contained in:
C. Scott Ananian 2013-09-02 14:51:23 -04:00 committed by Catrope
parent 8a2b55321c
commit 5830bce72c
4 changed files with 216 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
modules/ve/dm/ve.dm.Transaction.js
View file

@ -890,11 +890,22 @@ ve.dm.Transaction.prototype.pushReplace = function ( doc, offset, removeLength,
removeMetadata = metadataReplace.remove,
insertMetadata = metadataReplace.insert;
if ( lastOp && lastOp.type === 'replace' && !lastOp.removeMetadata && !removeMetadata ) {
// simple replaces can just be concatenated
// TODO: allow replaces with meta to be merged?
lastOp.insert = lastOp.insert.concat( insert );
lastOp.remove = lastOp.remove.concat( remove );
// simple replaces can be combined
// (but don't do this if there is metadata to be removed and the previous
// replace had a non-zero insertion, because that would shift the metadata
// location.)
if (
lastOp && lastOp.type === 'replace' &&
!( lastOp.insert.length > 0 && removeMetadata !== undefined )
) {
lastOp = this.operations.pop();
this.lengthDifference -= lastOp.insert.length - lastOp.remove.length;
this.pushReplace(
doc,
offset - lastOp.remove.length,
lastOp.remove.length + removeLength,
lastOp.insert.concat( insert )
);
} else {
op = {
'type': 'replace',
@ -906,8 +917,8 @@ ve.dm.Transaction.prototype.pushReplace = function ( doc, offset, removeLength,
op.insertMetadata = insertMetadata;
}
this.operations.push( op );
this.lengthDifference += insert.length - remove.length;
}
this.lengthDifference += insert.length - remove.length;
};
/**

64
modules/ve/test/dm/ve.dm.Transaction.test.js
View file

@ -1024,6 +1024,9 @@ QUnit.test( 'newFromContentBranchConversion', function ( assert ) {
QUnit.test( 'newFromWrap', function ( assert ) {
var i, key,
doc = ve.dm.example.createExampleDocument(),
metaDoc = ve.dm.example.createExampleDocument( 'withMeta' ),
listMetaDoc = ve.dm.example.createExampleDocument( 'listWithMeta' ),
listDoc = ve.dm.example.createExampleDocumentFromObject( 'listDoc', null, { 'listDoc': listMetaDoc.getData() } ),
cases = {
'changes a heading to a paragraph': {
'args': [doc, new ve.Range( 1, 4 ), [ { 'type': 'heading', 'attributes': { 'level': 1 } } ], [ { 'type': 'paragraph' } ], [], []],
@ -1048,6 +1051,25 @@ QUnit.test( 'newFromWrap', function ( assert ) {
{ 'type': 'retain', 'length': 37 }
]
},
'unwraps a multiple-item list': {
'args': [listDoc, new ve.Range( 1, 11 ), [ { 'type': 'list' } ], [], [ { 'type': 'listItem', 'attributes': {'styles': ['bullet']} } ], [] ],
'ops': [
{ 'type': 'replace',
'remove': [ { 'type': 'list' }, { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } } ],
'insert': []
},
{ 'type': 'retain', 'length': 3 },
{ 'type': 'replace',
'remove': [ { 'type': '/listItem' }, { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } } ],
'insert': []
},
{ 'type': 'retain', 'length': 3 },
{ 'type': 'replace',
'remove': [ { 'type': '/listItem' }, { 'type': '/list' } ],
'insert': []
}
]
},
'replaces a table with a list': {
'args': [doc, new ve.Range( 9, 33 ), [ { 'type': 'table' }, { 'type': 'tableSection', 'attributes': { 'style': 'body' } }, { 'type': 'tableRow' }, { 'type': 'tableCell' } ], [ { 'type': 'list' }, { 'type': 'listItem' } ], [], []],
'ops': [
@ -1094,6 +1116,48 @@ QUnit.test( 'newFromWrap', function ( assert ) {
{ 'type': 'retain', 'length': 2 }
]
},
'metadata is preserved on wrap': {
'args': [metaDoc, new ve.Range( 1, 10 ), [ { 'type': 'paragraph' } ], [ { 'type': 'heading', 'level': 1 } ], [], [] ],
'ops': [
{ 'type': 'replace',
'remove': [ { 'type': 'paragraph' } ],
'insert': [ { 'type': 'heading', 'level': 1 } ],
'insertMetadata': metaDoc.getMetadata().slice(0, 1),
'removeMetadata': metaDoc.getMetadata().slice(0, 1)
},
{ 'type': 'retain', 'length': 9 },
{ 'type': 'replace',
'remove': [ { 'type': '/paragraph' } ],
'insert': [ { 'type': '/heading' } ]
},
{ 'type': 'retain', 'length': 2 }
]
},
'metadata is preserved on unwrap': {
'args': [listMetaDoc, new ve.Range( 1, 11 ), [ { 'type': 'list' } ], [], [ { 'type': 'listItem', 'attributes': {'styles': ['bullet']} } ], [] ],
'ops': [
{ 'type': 'replace',
'remove': [ { 'type': 'list' }, { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } } ],
'insert': [],
'insertMetadata': ve.dm.MetaLinearData.static.merge( listMetaDoc.getMetadata().slice(0, 3) ),
'removeMetadata': listMetaDoc.getMetadata().slice(0, 3)
},
{ 'type': 'retain', 'length': 3 },
{ 'type': 'replace',
'remove': [ { 'type': '/listItem' }, { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } } ],
'insert': [],
'insertMetadata': ve.dm.MetaLinearData.static.merge( listMetaDoc.getMetadata().slice(5, 8) ),
'removeMetadata': listMetaDoc.getMetadata().slice(5, 8)
},
{ 'type': 'retain', 'length': 3 },
{ 'type': 'replace',
'remove': [ { 'type': '/listItem' }, { 'type': '/list' } ],
'insert': [],
'insertMetadata': ve.dm.MetaLinearData.static.merge( listMetaDoc.getMetadata().slice(10, 13) ),
'removeMetadata': listMetaDoc.getMetadata().slice(10, 13)
}
]
},
'checks integrity of unwrapOuter parameter': {
'args': [doc, new ve.Range( 13, 32 ), [ { 'type': 'table' } ], [], [], []],
'exception': Error

25
modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
View file

@ -452,6 +452,22 @@ QUnit.test( 'commit/rollback', function ( assert ) {
data.splice( 2, 2 );
data.splice( 4, 3 );
}
},
'preserves metadata on unwrap': {
'data': ve.dm.example.listWithMeta,
'calls': [
[ 'newFromWrap', new ve.Range( 1, 11 ),
[ { 'type': 'list' } ], [],
[ { 'type': 'listItem', 'attributes': {'styles': ['bullet']} } ], [] ]
],
'expected': function ( data ) {
data.splice( 35, 1 ); // remove '/list'
data.splice( 32, 1 ); // remove '/listItem'
data.splice( 20, 1 ); // remove 'listItem'
data.splice( 17, 1 ); // remove '/listItem'
data.splice( 5, 1 ); // remove 'listItem'
data.splice( 2, 1 ); // remove 'list'
}
}
};
@ -473,10 +489,15 @@ QUnit.test( 'commit/rollback', function ( assert ) {
tx = new ve.dm.Transaction();
for ( i = 0; i < cases[msg].calls.length; i++ ) {
// pushReplace needs the document as its first argument
if ( cases[msg].calls[i][0] === 'pushReplace' ) {
// some calls need the document as its first argument
if ( /^(pushReplace$|new)/.test( cases[msg].calls[i][0] ) ) {
cases[msg].calls[i].splice( 1, 0, testDoc );
}
// special case static methods of Transaction
if ( /^new/.test( cases[msg].calls[i][0] ) ) {
tx = ve.dm.Transaction[cases[msg].calls[i][0]].apply( null, cases[msg].calls[i].slice( 1 ) );
break;
}
tx[cases[msg].calls[i][0]].apply( tx, cases[msg].calls[i].slice( 1 ) );
}

112
modules/ve/test/dm/ve.dm.example.js
View file

@ -589,6 +589,118 @@ ve.dm.example.withMetaMetaData = [
];
ve.dm.example.listWithMeta = [
// 0 - Beginning of list
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="one" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': 'list' },
// 1 - Beginning of first list item
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="two" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } },
// 2 - Beginning of paragraph
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="three" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': 'paragraph' },
// 3 - Plain "a"
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="four" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
'a',
// 4 - End of paragraph
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="five" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': '/paragraph' },
// 5 - End of first list item
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="six" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': '/listItem' },
// 6 - Beginning of second list item
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="seven" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } },
// 7 - Beginning of paragraph
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="eight" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': 'paragraph' },
// 8 - Plain "b"
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="nine" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
'b',
// 9 - End of paragraph
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="ten" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': '/paragraph' },
// 10 - End of second list item
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="eleven" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': '/listItem' },
// 11 - End of list
{
'type': 'alienMeta',
'attributes': {
'domElements': $( '<meta property="twelve" />' ).toArray()
}
},
{ 'type': '/alienMeta' },
{ 'type': '/list' }
];
ve.dm.example.complexTableHtml = '<table><caption>Foo</caption><thead><tr><th>Bar</th></tr></thead>' +
'<tfoot><tr><td>Baz</td></tr></tfoot><tbody><tr><td>Quux</td><td>Whee</td></tr></tbody></table>';