From 2f18605a1a5f57a0c5340f79d9ab4b48fb0aeda9 Mon Sep 17 00:00:00 2001 From: Catrope Date: Thu, 31 May 2012 06:23:44 -0700 Subject: [PATCH] Fix buggy replace behavior when inserting content that contains nodes Copy-pasting things like "textmoretext" 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 --- modules/ve2/dm/ve.dm.TransactionProcessor.js | 31 ++++++++++++++----- .../ve2/dm/ve.dm.TransactionProcessor.test.js | 15 +++++++-- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/modules/ve2/dm/ve.dm.TransactionProcessor.js b/modules/ve2/dm/ve.dm.TransactionProcessor.js index 2f9a25017c..648c5dc39c 100644 --- a/modules/ve2/dm/ve.dm.TransactionProcessor.js +++ b/modules/ve2/dm/ve.dm.TransactionProcessor.js @@ -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 { diff --git a/tests/ve2/dm/ve.dm.TransactionProcessor.test.js b/tests/ve2/dm/ve.dm.TransactionProcessor.test.js index 4f0ee5fdd4..cd78fd27e6 100644 --- a/tests/ve2/dm/ve.dm.TransactionProcessor.test.js +++ b/tests/ve2/dm/ve.dm.TransactionProcessor.test.js @@ -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': [ [