r103535 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103534‎ | r103535 | r103536 >
Date:00:26, 18 November 2011
Author:tparscal
Status:ok (Comments)
Tags:
Comment:
Added first node preservation when rebuilding
Modified paths:
  • /trunk/extensions/VisualEditor/modules/es/es.TransactionProcessor.js (modified) (history)

Diff [purge]

Index: trunk/extensions/VisualEditor/modules/es/es.TransactionProcessor.js
@@ -89,7 +89,8 @@
9090 };
9191
9292 es.TransactionProcessor.prototype.rebuildNodes = function( newData, oldNodes, parent, index ) {
93 - var remove = 0;
 93+ var newNodes = es.DocumentModel.createNodesFromData( newData ),
 94+ remove = 0;
9495 if ( oldNodes ) {
9596 if ( oldNodes[0] === oldNodes[0].getRoot() ) {
9697 parent = oldNodes[0];
@@ -100,13 +101,34 @@
101102 index = parent.indexOf( oldNodes[0] );
102103 remove = oldNodes.length;
103104 }
 105+ // Try to preserve the first node
 106+ if (
 107+ // There must be an old and new node to preserve
 108+ newNodes.length &&
 109+ oldNodes.length &&
 110+ // Node types need to match
 111+ newNodes[0].type === oldNodes[0].type &&
 112+ // Only for leaf nodes
 113+ !newNodes[0].hasChildren()
 114+ ) {
 115+ var newNode = newNodes.shift(),
 116+ oldNode = oldNodes.shift();
 117+ // Let's just leave this first node in place and adjust it's length
 118+ var newAttributes = newNode.getElement().attributes,
 119+ oldAttributes = oldNode.getElement().attributes;
 120+ if ( oldAttributes || newAttributes ) {
 121+ oldNode.getElement().attributes = newAttributes;
 122+ }
 123+ oldNode.adjustContentLength( newNode.getContentLength() - oldNode.getContentLength() );
 124+ index++;
 125+ remove--;
 126+ }
104127 }
105128 // Try to perform this in a single operation if possible, this reduces the number of UI updates
106129 // TODO: Introduce a global for max argument length - 1024 is also assumed in es.insertIntoArray
107 - var newNodes = es.DocumentModel.createNodesFromData( newData );
108130 if ( newNodes.length < 1024 ) {
109131 parent.splice.apply( parent, [index, remove].concat( newNodes ) );
110 - } else {
 132+ } else if ( newNodes.length ) {
111133 parent.splice.apply( parent, [index, remove] );
112134 // Safe to call with arbitrary length of newNodes
113135 es.insertIntoArray( parent, index, newNodes );

Comments

#Comment by Catrope (talk | contribs)   15:06, 30 November 2011

What happens if, due to the optimization, newNodes is empty and remove becomes zero? Won't that cause splice() to fail? (The docs for splice() say it may fail in some browsers if you tell it to remove nothing and add nothing.)

OK otherwise, marking OK.

Status & tagging log