| Index: trunk/extensions/VisualEditor/tests/es/es.TransactionProcessor.test.js |
| — | — | @@ -1,6 +1,6 @@ |
| 2 | 2 | module( 'es' ); |
| 3 | 3 | |
| 4 | | -test( 'es.TransactionProcessor', 18, function() { |
| | 4 | +test( 'es.TransactionProcessor', 29, function() { |
| 5 | 5 | var documentModel = es.DocumentModel.newFromPlainObject( esTest.obj ); |
| 6 | 6 | |
| 7 | 7 | // FIXME: These tests shouldn't use prepareFoo() because those functions |
| — | — | @@ -243,4 +243,109 @@ |
| 244 | 244 | 'table', |
| 245 | 245 | 'rollback keeps model tree up to date with paragraph split (table follows the paragraph)' |
| 246 | 246 | ); |
| | 247 | + |
| | 248 | + var listItemMerge = documentModel.prepareRemoval( new es.Range( 14, 19 ) ); |
| | 249 | + |
| | 250 | + // Test 19 |
| | 251 | + es.TransactionProcessor.commit( documentModel, listItemMerge ); |
| | 252 | + deepEqual( |
| | 253 | + documentModel.getData( new es.Range( 12, 22 ) ), |
| | 254 | + [ |
| | 255 | + { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } }, |
| | 256 | + { 'type': 'paragraph' }, |
| | 257 | + 'f', |
| | 258 | + { 'type': '/paragraph' }, |
| | 259 | + { 'type': '/listItem' }, |
| | 260 | + { 'type': 'listItem', 'attributes': { 'styles': ['number'] } }, |
| | 261 | + { 'type': 'paragraph' }, |
| | 262 | + 'g', |
| | 263 | + { 'type': '/paragraph' }, |
| | 264 | + { 'type': '/listItem' } |
| | 265 | + ], |
| | 266 | + 'removal merges two list items with paragraphs' |
| | 267 | + ); |
| | 268 | + |
| | 269 | + // Test 20 |
| | 270 | + deepEqual( documentModel.children[1].children[0].children[0].children[1].children.length, 2, |
| | 271 | + 'removal keeps model tree up to date with list item merge (number of children)' |
| | 272 | + ); |
| | 273 | + |
| | 274 | + // Test 21 |
| | 275 | + deepEqual( |
| | 276 | + documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContent(), |
| | 277 | + [ 'f' ], |
| | 278 | + 'removal keeps model tree up to date with list item merge (first list item)' |
| | 279 | + ); |
| | 280 | + |
| | 281 | + // Test 22 |
| | 282 | + deepEqual( |
| | 283 | + documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContent(), |
| | 284 | + [ 'g' ], |
| | 285 | + 'removal keeps model tree up to date with list item merge (second list item)' |
| | 286 | + ); |
| | 287 | + |
| | 288 | + // Test 23 |
| | 289 | + deepEqual( |
| | 290 | + documentModel.children[2].getContent(), |
| | 291 | + [ 'h' ], |
| | 292 | + 'rollback keeps model tree up to date with list item split (final paragraph)' |
| | 293 | + ); |
| | 294 | + |
| | 295 | + // Test 24 |
| | 296 | + es.TransactionProcessor.rollback( documentModel, listItemMerge ); |
| | 297 | + deepEqual( |
| | 298 | + documentModel.getData( new es.Range( 12, 27 ) ), |
| | 299 | + [ |
| | 300 | + { 'type': 'listItem', 'attributes': { 'styles': ['bullet'] } }, |
| | 301 | + { 'type': 'paragraph' }, |
| | 302 | + 'e', |
| | 303 | + { 'type': '/paragraph' }, |
| | 304 | + { 'type': '/listItem' }, |
| | 305 | + { 'type': 'listItem', 'attributes': { 'styles': ['bullet', 'bullet'] } }, |
| | 306 | + { 'type': 'paragraph' }, |
| | 307 | + 'f', |
| | 308 | + { 'type': '/paragraph' }, |
| | 309 | + { 'type': '/listItem' }, |
| | 310 | + { 'type': 'listItem', 'attributes': { 'styles': ['number'] } }, |
| | 311 | + { 'type': 'paragraph' }, |
| | 312 | + 'g', |
| | 313 | + { 'type': '/paragraph' }, |
| | 314 | + { 'type': '/listItem' } |
| | 315 | + ], |
| | 316 | + 'rollback reverses list item merge (splits the list items)' |
| | 317 | + ); |
| | 318 | + |
| | 319 | + // Test 25 |
| | 320 | + deepEqual( documentModel.children[1].children[0].children[0].children[1].children.length, 3, |
| | 321 | + 'rollback keeps model tree up to date with list item split (number of children)' |
| | 322 | + ); |
| | 323 | + |
| | 324 | + // Test 26 |
| | 325 | + deepEqual( |
| | 326 | + documentModel.children[1].children[0].children[0].children[1].children[0].children[0].getContent(), |
| | 327 | + [ 'e' ], |
| | 328 | + 'rollback keeps model tree up to date with list item split (first list item)' |
| | 329 | + ); |
| | 330 | + |
| | 331 | + // Test 27 |
| | 332 | + deepEqual( |
| | 333 | + documentModel.children[1].children[0].children[0].children[1].children[1].children[0].getContent(), |
| | 334 | + [ 'f' ], |
| | 335 | + 'rollback keeps model tree up to date with list item split (second list item)' |
| | 336 | + ); |
| | 337 | + |
| | 338 | + // Test 28 |
| | 339 | + deepEqual( |
| | 340 | + documentModel.children[1].children[0].children[0].children[1].children[2].children[0].getContent(), |
| | 341 | + [ 'g' ], |
| | 342 | + 'rollback keeps model tree up to date with list item split (third list item)' |
| | 343 | + ); |
| | 344 | + |
| | 345 | + // Test 29 |
| | 346 | + deepEqual( |
| | 347 | + documentModel.children[2].getContent(), |
| | 348 | + [ 'h' ], |
| | 349 | + 'rollback keeps model tree up to date with list item split (final paragraph)' |
| | 350 | + ); |
| | 351 | + |
| 247 | 352 | } ); |
| Index: trunk/extensions/VisualEditor/modules/es/es.TransactionProcessor.js |
| — | — | @@ -88,17 +88,23 @@ |
| 89 | 89 | } |
| 90 | 90 | }; |
| 91 | 91 | |
| | 92 | +// TODO: document this. Various arguments are optional or nonoptional in different cases, that's confusing |
| | 93 | +// so it needs to be documented well. |
| 92 | 94 | es.TransactionProcessor.prototype.rebuildNodes = function( newData, oldNodes, parent, index ) { |
| 93 | 95 | var newNodes = es.DocumentModel.createNodesFromData( newData ), |
| 94 | 96 | remove = 0; |
| 95 | 97 | if ( oldNodes ) { |
| | 98 | + // Determine parent and index if not given |
| 96 | 99 | if ( oldNodes[0] === oldNodes[0].getRoot() ) { |
| | 100 | + // We know the values for parent and index in this case |
| | 101 | + // and don't have to compute them. Override any parent |
| | 102 | + // or index parameter passed. |
| 97 | 103 | parent = oldNodes[0]; |
| 98 | 104 | index = 0; |
| 99 | 105 | remove = parent.getChildren().length; |
| 100 | 106 | } else { |
| 101 | | - parent = oldNodes[0].getParent(); |
| 102 | | - index = parent.indexOf( oldNodes[0] ); |
| | 107 | + parent = parent || oldNodes[0].getParent(); |
| | 108 | + index = index || parent.indexOf( oldNodes[0] ); |
| 103 | 109 | remove = oldNodes.length; |
| 104 | 110 | } |
| 105 | 111 | // Try to preserve the first node |
| — | — | @@ -264,16 +270,16 @@ |
| 265 | 271 | if ( es.DocumentModel.containsElementData( op.data ) ) { |
| 266 | 272 | // Figure out which nodes are covered by the removal |
| 267 | 273 | var ranges = this.model.selectNodes( new es.Range( this.cursor, this.cursor + op.data.length ) ); |
| 268 | | - var oldNodes = [], newData = [], firstKeptNode = true, lastElement; |
| | 274 | + |
| | 275 | + // Build the list of nodes to rebuild and the data to keep |
| | 276 | + var oldNodes = [], newData = [], parent = null, index = null, firstKeptNode, lastKeptNode; |
| 269 | 277 | for ( var i = 0; i < ranges.length; i++ ) { |
| 270 | 278 | oldNodes.push( ranges[i].node ); |
| 271 | 279 | if ( ranges[i].range !== undefined ) { |
| 272 | 280 | // We have to keep part of this node |
| 273 | | - if ( firstKeptNode ) { |
| | 281 | + if ( firstKeptNode === undefined ) { |
| 274 | 282 | // This is the first node we're keeping |
| 275 | | - // Keep its opening as well |
| 276 | | - newData.push( ranges[i].node.getElement() ); |
| 277 | | - firstKeptNode = false; |
| | 283 | + firstKeptNode = ranges[i].node; |
| 278 | 284 | } |
| 279 | 285 | // Compute the start and end offset of this node |
| 280 | 286 | // We could do that with getOffsetFromNode() but |
| — | — | @@ -288,17 +294,76 @@ |
| 289 | 295 | // Append it to newData |
| 290 | 296 | newData = newData.concat( nodeData ); |
| 291 | 297 | |
| 292 | | - lastElement = ranges[i].node.getElementType(); |
| | 298 | + lastKeptNode = ranges[i].node; |
| 293 | 299 | } |
| 294 | 300 | } |
| 295 | | - if ( lastElement !== undefined ) { |
| 296 | | - // Keep the closing of the last element that was partially kept |
| 297 | | - newData.push( { 'type': '/' + lastElement } ); |
| | 301 | + |
| | 302 | + // Surround newData with the right openings and closings if needed |
| | 303 | + if ( firstKeptNode !== undefined ) { |
| | 304 | + // There are a number of conceptually different cases here, |
| | 305 | + // but the algorithm for dealing with them is the same. |
| | 306 | + // 1. Removal within one node: firstKeptNode === lastKeptNode |
| | 307 | + // 2. Merge of siblings: firstKeptNode.getParent() === lastKeptNode.getParent() |
| | 308 | + // 3. Merge of arbitrary depth: firstKeptNode and lastKeptNode have a common ancestor |
| | 309 | + // Because #1 and #2 are special cases of #3 (merges with depth=0 and depth=1, respectively), |
| | 310 | + // the code below that deals with the general case (#3) and automatically covers |
| | 311 | + // #1 and #2 that way as well. |
| | 312 | + |
| | 313 | + // Simultaneously traverse upwards from firstKeptNode and lastKeptNode |
| | 314 | + // to find the common ancestor. On our way up, keep the element of each |
| | 315 | + // node we visit and verify that the transaction is a valid merge (i.e. it satisfies |
| | 316 | + // the merge criteria in prepareRemoval()'s canMerge()). |
| | 317 | + // FIXME: The code is essentially the same as canMerge(), merge these algorithms |
| | 318 | + var n1 = firstKeptNode, n2 = lastKeptNode, |
| | 319 | + prevN1 = null, prevN2 = null, |
| | 320 | + openings = [], closings = []; |
| | 321 | + while ( n1 !== n2 ) { |
| | 322 | + // Verify the element types are equal |
| | 323 | + if ( n1.getElementType() !== n2.getElementType() ) { |
| | 324 | + throw 'Removal is not a valid merge: corresponding parents have different types ( ' + |
| | 325 | + n1.getElementType() + ' vs ' + n2.getElementType() + ' )'; |
| | 326 | + } |
| | 327 | + // Record the opening of n1 and the closing of n2 |
| | 328 | + openings.push( n1.getElement() ); |
| | 329 | + closings.push( { 'type': '/' + n2.getElementType() } ); |
| | 330 | + // Move up |
| | 331 | + prevN1 = n1; |
| | 332 | + prevN2 = n2; |
| | 333 | + n1 = n1.getParent(); |
| | 334 | + n2 = n2.getParent(); |
| | 335 | + if ( n1 === null || n2 === null ) { |
| | 336 | + // Reached a root, so no common ancestor or different depth |
| | 337 | + throw 'Removal is not a valid merge: nodes do not have a common ancestor or are not at the same depth'; |
| | 338 | + } |
| | 339 | + } |
| | 340 | + |
| | 341 | + // Surround newData with the openings and closings |
| | 342 | + newData = openings.reverse().concat( newData, closings ); |
| | 343 | + |
| | 344 | + // Rebuild oldNodes if needed |
| | 345 | + // This only happens for merges with depth > 1 |
| | 346 | + if ( prevN1 !== oldNodes[0] ) { |
| | 347 | + oldNodes = [ prevN1 ]; |
| | 348 | + parent = n1; |
| | 349 | + index = n1.indexOf( prevN1 ); // Pass to rebuildNodes() so it's not recomputed |
| | 350 | + if ( index === -1 ) { |
| | 351 | + throw "Tree corruption detected: node isn't in its parent's children array"; |
| | 352 | + } |
| | 353 | + var foundPrevN2 = false; |
| | 354 | + for ( var j = index + 1; !foundPrevN2 && j < n1.getChildren().length; j++ ) { |
| | 355 | + oldNodes.push( n1.getChildren()[j] ); |
| | 356 | + foundPrevN2 = n1.getChildren()[j] === prevN2; |
| | 357 | + } |
| | 358 | + if ( !foundPrevN2 ) { |
| | 359 | + throw "Tree corruption detected: node isn't in its parent's children array"; |
| | 360 | + } |
| | 361 | + } |
| 298 | 362 | } |
| | 363 | + |
| 299 | 364 | // Update the linear model |
| 300 | 365 | this.model.data.splice( this.cursor, op.data.length ); |
| 301 | 366 | // Perform the rebuild. This updates the model tree |
| 302 | | - this.rebuildNodes( newData, oldNodes ); |
| | 367 | + this.rebuildNodes( newData, oldNodes, parent, index ); |
| 303 | 368 | } else { |
| 304 | 369 | // We're removing content only. Take a shortcut |
| 305 | 370 | // Get the node we are removing content from |