From 29f416937e2eb2088074e12b16eec569823509f8 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Sat, 10 Mar 2012 00:31:28 +0000 Subject: [PATCH] Fix some usages of splice.apply in the data model to use ve.batchedSplice(). Added FIXME comments for occurrences outside of DM --- demos/ce/main.js | 2 ++ modules/parser/mediawiki.TokenTransformManager.js | 4 ++++ modules/sandbox/sandbox.js | 2 ++ modules/ve/dm/ve.dm.DocumentSynchronizer.js | 2 +- modules/ve/dm/ve.dm.TransactionProcessor.js | 10 +--------- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/demos/ce/main.js b/demos/ce/main.js index 5cb0d539e3..44e8ba8be3 100644 --- a/demos/ce/main.js +++ b/demos/ce/main.js @@ -625,6 +625,8 @@ $(document).ready( function() { documentModel.data.splice( 0, documentModel.data.length ); ve.insertIntoArray( documentModel.data, 0, newDocumentModel.data ); surfaceModel.select( new ve.Range( 1, 1 ) ); + // FIXME: this should be using ve.batchedSplice(), otherwise things + // could explode if newDocumentModel.getChildren() is very long documentModel.splice.apply( documentModel, [0, documentModel.getChildren().length] diff --git a/modules/parser/mediawiki.TokenTransformManager.js b/modules/parser/mediawiki.TokenTransformManager.js index 632feb478d..79faff00f6 100644 --- a/modules/parser/mediawiki.TokenTransformManager.js +++ b/modules/parser/mediawiki.TokenTransformManager.js @@ -467,6 +467,8 @@ AsyncTokenTransformManager.prototype.transformTokens = function ( tokens, parent if( res.tokens ) { // Splice in the returned tokens (while replacing the original // token), and process them next. + // FIXME: this should be using ve.batchedSplice(), otherwise things + // could explode if res.tokens is very long [].splice.apply( tokens, [i, 1].concat(res.tokens) ); tokensLength = tokens.length; i--; // continue at first inserted token @@ -662,6 +664,8 @@ SyncTokenTransformManager.prototype.onChunk = function ( tokens ) { if( res.tokens ) { // Splice in the returned tokens (while replacing the original // token), and process them next. + // FIXME: this should be using ve.batchedSplice(), otherwise things + // could explode if res.tokens is very long [].splice.apply( tokens, [i, 1].concat(res.tokens) ); tokensLength = tokens.length; i--; // continue at first inserted token diff --git a/modules/sandbox/sandbox.js b/modules/sandbox/sandbox.js index 19ba2cc2e5..743c6ad829 100644 --- a/modules/sandbox/sandbox.js +++ b/modules/sandbox/sandbox.js @@ -717,6 +717,8 @@ $(document).ready( function() { documentModel.data.splice( 0, documentModel.data.length ); ve.insertIntoArray( documentModel.data, 0, newDocumentModel.data ); surfaceModel.select( new ve.Range( 1, 1 ) ); + // FIXME: this should be using ve.batchedSplice(), otherwise things + // could explode if newDocumentModel.getChildren() is very long documentModel.splice.apply( documentModel, [0, documentModel.getChildren().length] diff --git a/modules/ve/dm/ve.dm.DocumentSynchronizer.js b/modules/ve/dm/ve.dm.DocumentSynchronizer.js index 86e9ff0edc..ca2b9cc894 100644 --- a/modules/ve/dm/ve.dm.DocumentSynchronizer.js +++ b/modules/ve/dm/ve.dm.DocumentSynchronizer.js @@ -86,7 +86,7 @@ ve.dm.DocumentSynchronizer.prototype.synchronize = function() { new ve.Range( offset, action.node.getElementLength() + action.adjustment ) ) ); parent = action.node.getParent(); - parent.splice.apply( parent, [parent.indexOf( action.node ), 1].concat( newNodes ) ); + ve.batchedSplice( parent, parent.indexOf( action.node ), 1, newNodes ); // Adjust proceeding offsets by the difference between the original and new nodes var newNodesLength = 0; for ( var j = 0, jlen = newNodes.length; j < jlen; j++ ) { diff --git a/modules/ve/dm/ve.dm.TransactionProcessor.js b/modules/ve/dm/ve.dm.TransactionProcessor.js index 3418a7bdab..26bee2bd39 100644 --- a/modules/ve/dm/ve.dm.TransactionProcessor.js +++ b/modules/ve/dm/ve.dm.TransactionProcessor.js @@ -138,15 +138,7 @@ ve.dm.TransactionProcessor.prototype.rebuildNodes = function( newData, oldNodes, remove--; } } - // Try to perform this in a single operation if possible, this reduces the number of UI updates - // TODO: Introduce a global for max argument length - 1024 is also assumed in ve.insertIntoArray - if ( newNodes.length < 1024 ) { - parent.splice.apply( parent, [index, remove].concat( newNodes ) ); - } else if ( newNodes.length ) { - parent.splice.apply( parent, [index, remove] ); - // Safe to call with arbitrary length of newNodes - ve.insertIntoArray( parent, index, newNodes ); - } + ve.batchedSplice( parent, index, remove, newNodes ); }; /**