wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-11-15 10:35:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix buggy replace behavior when inserting content that contains nodes

Browse source 
Copy-pasting things like "text<IMAGE>moretext" failed spectacularly,
this commit fixes that.

* Check for content rather than structure in the inserted/removed data
* In the content case
** Run selectNodes() over the removal range, rather than just the cursor
*** i.e. no longer assume that content replacements only affect one node
** If there is structure involved, rebuild all affected nodes

Change-Id: I80e40b5b7c514a3fb105d57e4a17770d0fefaaea
This commit is contained in:
Catrope 2012-05-31 06:23:44 -07:00
parent f6ca37926d
commit 2f18605a1a
2 changed files with 35 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
modules/ve2/dm/ve.dm.TransactionProcessor.js
View file

@ -254,23 +254,38 @@ ve.dm.TransactionProcessor.prototype.attribute = function( op ) {
ve.dm.TransactionProcessor.prototype.replace = function( op ) {
var remove = this.reversed ? op.insert : op.remove,
insert = this.reversed ? op.remove : op.insert,
removeHasStructure = ve.dm.Document.containsElementData( remove ),
insertHasStructure = ve.dm.Document.containsElementData( insert ),
removeIsContent = ve.dm.Document.isContentData( remove ),
insertIsContent = ve.dm.Document.isContentData( insert ),
node, selection;
// Figure out if this is a structural insert or a content insert
if ( !removeHasStructure && !insertHasStructure ) {
if ( removeIsContent && insertIsContent ) {
// Content replacement
// Update the linear model
ve.batchSplice( this.document.data, this.cursor, remove.length, insert );
this.applyAnnotations( this.cursor + insert.length );
// Get the node containing the replaced content
selection = this.document.selectNodes( new ve.Range( this.cursor, this.cursor ),
selection = this.document.selectNodes(
new ve.Range( this.cursor, this.cursor + remove.length ),
'leaves'
);
node = selection[0].node;
// Queue a resize for this node
this.synchronizer.pushResize( node, insert.length - remove.length );
var removeHasStructure = ve.dm.Document.containsElementData( remove ),
insertHasStructure = ve.dm.Document.containsElementData( insert );
if ( removeHasStructure || insertHasStructure ) {
// Replacement is not exclusively text
// Rebuild all covered nodes
var range = new ve.Range( selection[0].nodeRange.start,
selection[selection.length - 1].nodeRange.end );
this.synchronizer.pushRebuild( range,
new ve.Range( range.start, range.end + insert.length - remove.length )
);
} else {
// Text-only replacement
// Queue a resize for this node
node = selection[0].node;
this.synchronizer.pushResize( node, insert.length - remove.length );
}
// Advance the cursor
this.cursor += insert.length;
} else {

15
tests/ve2/dm/ve.dm.TransactionProcessor.test.js
View file

@ -105,7 +105,7 @@ test( 'commit/rollback', function() {
],
'exception': /^Invalid element error, can not set attributes on non-element data$/
},
'inserting content': {
'inserting text': {
'calls': [
['pushRetain', 1],
['pushReplace', [], ['F', 'O', 'O']]
@ -114,7 +114,7 @@ test( 'commit/rollback', function() {
data.splice( 1, 0, 'F', 'O', 'O' );
}
},
'removing content': {
'removing text': {
'calls': [
['pushRetain', 1],
['pushReplace', ['a'], []]
@ -123,7 +123,7 @@ test( 'commit/rollback', function() {
data.splice( 1, 1 );
}
},
'replacing content': {
'replacing text': {
'calls': [
['pushRetain', 1],
['pushReplace', ['a'], ['F', 'O', 'O']]
@ -132,6 +132,15 @@ test( 'commit/rollback', function() {
data.splice( 1, 1, 'F', 'O', 'O' );
}
},
'inserting mixed content': {
'calls': [
['pushRetain', 1],
['pushReplace', ['a'], ['F', 'O', 'O', {'type':'image'}, {'type':'/image'}, 'B', 'A', 'R']]
],
'expected': function( data ) {
data.splice( 1, 1, 'F', 'O', 'O', {'type':'image'}, {'type':'/image'}, 'B', 'A', 'R' );
}
},
'converting an element': {
'calls': [
[