| Index: trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js |
| — | — | @@ -958,25 +958,18 @@ |
| 959 | 959 | // a table row. There may be other rules we will want in here later, for instance, special |
| 960 | 960 | // casing merging a listitem into a paragraph. |
| 961 | 961 | function canMerge( node1, node2 ) { |
| 962 | | - var n1 = node1, n2 = node2; |
| 963 | | - // Simultaneously walk upwards from node1 and node2 |
| 964 | | - // until we reach the common ancestor. |
| 965 | | - while ( n1 !== n2 ) { |
| 966 | | - if ( n1.getElementType() !== n2.getElementType() ) { |
| 967 | | - // Not the same type |
| | 962 | + var result = es.DocumentNode.getCommonAncestorPaths( node1, node2 ); |
| | 963 | + if ( !result ) { |
| | 964 | + return false; |
| | 965 | + } |
| | 966 | + |
| | 967 | + // Check that corresponding nodes in the paths have the same type |
| | 968 | + for ( var i = 0; i < result.node1Path.length; i++ ) { |
| | 969 | + if ( result.node1Path[i].getElementType() !== result.node2Path[i].getElementType() ) { |
| 968 | 970 | return false; |
| 969 | 971 | } |
| 970 | | - n1 = n1.getParent(); |
| 971 | | - n2 = n2.getParent(); |
| 972 | | - if ( n1 === null || n2 === null ) { |
| 973 | | - // Reached a root, so no common ancestor |
| 974 | | - // or different depth |
| 975 | | - return false; |
| 976 | | - } |
| 977 | 972 | } |
| 978 | | - // We've reached the common ancestor using simultaneous traversal, |
| 979 | | - // so we know node1 and node2 have the same depth. We also haven't |
| 980 | | - // seen any nodes with mismatching types along the way, so we're good. |
| | 973 | + |
| 981 | 974 | return true; |
| 982 | 975 | } |
| 983 | 976 | |
| Index: trunk/extensions/VisualEditor/modules/es/es.TransactionProcessor.js |
| — | — | @@ -326,27 +326,22 @@ |
| 327 | 327 | // node we visit and verify that the transaction is a valid merge (i.e. it satisfies |
| 328 | 328 | // the merge criteria in prepareRemoval()'s canMerge()). |
| 329 | 329 | // FIXME: The code is essentially the same as canMerge(), merge these algorithms |
| 330 | | - var n1 = firstKeptNode, n2 = lastKeptNode, |
| 331 | | - prevN1 = null, prevN2 = null, |
| 332 | | - openings = [], closings = []; |
| 333 | | - while ( n1 !== n2 ) { |
| | 330 | + var openings = [], closings = [], |
| | 331 | + paths = es.DocumentNode.getCommonAncestorPaths( firstKeptNode, lastKeptNode ), |
| | 332 | + i, prevN1, prevN2; |
| | 333 | + |
| | 334 | + if ( !paths ) { |
| | 335 | + throw 'Removal is not a valid merge: nodes do not have a common ancestor or are not at the same depth'; |
| | 336 | + } |
| | 337 | + for ( i = 0; i < paths.node1Path.length; i++ ) { |
| 334 | 338 | // Verify the element types are equal |
| 335 | | - if ( n1.getElementType() !== n2.getElementType() ) { |
| | 339 | + if ( paths.node1Path[i].getElementType() !== paths.node2Path[i].getElementType() ) { |
| 336 | 340 | throw 'Removal is not a valid merge: corresponding parents have different types ( ' + |
| 337 | | - n1.getElementType() + ' vs ' + n2.getElementType() + ' )'; |
| | 341 | + paths.node1Path[i].getElementType() + ' vs ' + paths.node2Path[i].getElementType() + ' )'; |
| 338 | 342 | } |
| 339 | 343 | // Record the opening of n1 and the closing of n2 |
| 340 | | - openings.push( n1.getElement() ); |
| 341 | | - closings.push( { 'type': '/' + n2.getElementType() } ); |
| 342 | | - // Move up |
| 343 | | - prevN1 = n1; |
| 344 | | - prevN2 = n2; |
| 345 | | - n1 = n1.getParent(); |
| 346 | | - n2 = n2.getParent(); |
| 347 | | - if ( n1 === null || n2 === null ) { |
| 348 | | - // Reached a root, so no common ancestor or different depth |
| 349 | | - throw 'Removal is not a valid merge: nodes do not have a common ancestor or are not at the same depth'; |
| 350 | | - } |
| | 344 | + openings.push( paths.node1Path[i].getElement() ); |
| | 345 | + closings.push( { 'type': '/' + paths.node2Path[i].getElementType() } ); |
| 351 | 346 | } |
| 352 | 347 | |
| 353 | 348 | // Surround newData with the openings and closings |
| — | — | @@ -354,17 +349,19 @@ |
| 355 | 350 | |
| 356 | 351 | // Rebuild oldNodes if needed |
| 357 | 352 | // This only happens for merges with depth > 1 |
| 358 | | - if ( prevN1 !== oldNodes[0] ) { |
| | 353 | + prevN1 = paths.node1Path.length ? paths.node1Path[paths.node1Path.length - 1] : null; |
| | 354 | + prevN2 = paths.node2Path.length ? paths.node2Path[paths.node2Path.length - 1] : null; |
| | 355 | + if ( prevN1 && prevN1 !== oldNodes[0] ) { |
| 359 | 356 | oldNodes = [ prevN1 ]; |
| 360 | | - parent = n1; |
| 361 | | - index = n1.indexOf( prevN1 ); // Pass to rebuildNodes() so it's not recomputed |
| | 357 | + parent = paths.commonAncestor; |
| | 358 | + index = parent.indexOf( prevN1 ); // Pass to rebuildNodes() so it's not recomputed |
| 362 | 359 | if ( index === -1 ) { |
| 363 | 360 | throw "Tree corruption detected: node isn't in its parent's children array"; |
| 364 | 361 | } |
| 365 | 362 | var foundPrevN2 = false; |
| 366 | | - for ( var j = index + 1; !foundPrevN2 && j < n1.getChildren().length; j++ ) { |
| 367 | | - oldNodes.push( n1.getChildren()[j] ); |
| 368 | | - foundPrevN2 = n1.getChildren()[j] === prevN2; |
| | 363 | + for ( var j = index + 1; !foundPrevN2 && j < parent.getChildren().length; j++ ) { |
| | 364 | + oldNodes.push( parent.getChildren()[j] ); |
| | 365 | + foundPrevN2 = parent.getChildren()[j] === prevN2; |
| 369 | 366 | } |
| 370 | 367 | if ( !foundPrevN2 ) { |
| 371 | 368 | throw "Tree corruption detected: node isn't in its parent's children array"; |