jenkins-bot has submitted this change and it was merged.
Change subject: Actually use op.retainMetadata in structural replace mode
......................................................................
Actually use op.retainMetadata in structural replace mode
It was being used correctly in the textual replace case, but it was
omitted in the structural replace case. This caused rare, insidious
metadata placement bugs that I haven't been able to reproduce outside
of newFromDocumentInsertion testing, but this might explain some of
the odd category bugs that have been reported.
Change-Id: I1424e482303853f285e4516a93c9609076648eff
---
M modules/ve/dm/ve.dm.MetaList.js
M modules/ve/dm/ve.dm.TransactionProcessor.js
M modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
3 files changed, 24 insertions(+), 2 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/dm/ve.dm.MetaList.js b/modules/ve/dm/ve.dm.MetaList.js
index 861b06a..78e8af4 100644
--- a/modules/ve/dm/ve.dm.MetaList.js
+++ b/modules/ve/dm/ve.dm.MetaList.js
@@ -96,7 +96,7 @@
index = 0;
break;
case 'replace':
- // if we have metadata replace info we can
calulcate the new
+ // if we have metadata replace info we can
calculate the new
// offset and index directly
ins = reversed ? ops[i].removeMetadata :
ops[i].insertMetadata;
rm = reversed ? ops[i].insertMetadata :
ops[i].removeMetadata;
diff --git a/modules/ve/dm/ve.dm.TransactionProcessor.js
b/modules/ve/dm/ve.dm.TransactionProcessor.js
index 9a8f86d..5e628b3 100644
--- a/modules/ve/dm/ve.dm.TransactionProcessor.js
+++ b/modules/ve/dm/ve.dm.TransactionProcessor.js
@@ -387,7 +387,7 @@
this.document.data.batchSplice( this.cursor,
opRemove.length, opInsert );
// Keep the meta linear model in sync
if ( opRemoveMetadata !== undefined ) {
- this.document.metadata.batchSplice(
this.cursor, opRemoveMetadata.length, opInsertMetadata );
+ this.document.metadata.batchSplice(
this.cursor + op.retainMetadata, opRemoveMetadata.length, opInsertMetadata );
} else if ( opInsert.length > opRemove.length )
{
this.document.metadata.batchSplice(
this.cursor + opRemove.length, 0, new Array( opInsert.length - opRemove.length
) );
} else if ( opInsert.length < opRemove.length )
{
diff --git a/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
b/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
index d461358..b52623a 100644
--- a/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
+++ b/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
@@ -315,6 +315,28 @@
'expected': function ( data ) {
data.splice( 15, 2 );
}
+ },
+ 'structural replacement starting at an offset without
metadata': {
+ 'data': [
+ { 'type': 'paragraph' },
+ 'F',
+ {
+ 'type': 'alienMeta',
+ 'attributes': {
+ 'domElements': $( '<!--
foo -->' ).toArray()
+ }
+ },
+ { 'type': '/alienMeta' },
+ 'o', 'o',
+ { 'type': '/paragraph' }
+ ],
+ 'calls': [
+ ['pushReplace', 0, 5, [ { 'type':
'table' }, { 'type': '/table' } ]]
+ ],
+ 'expected': function ( data ) {
+ data.splice( 0, 2, { 'type': 'table' },
{ 'type': '/table' } );
+ data.splice( 4, 3 );
+ }
}
};
--
To view, visit https://gerrit.wikimedia.org/r/73533
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1424e482303853f285e4516a93c9609076648eff
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
