| Index: trunk/extensions/VisualEditor/tests/es/es.DocumentModel.test.js |
| — | — | @@ -375,15 +375,14 @@ |
| 376 | 376 | 'prepareRemoval works across structural nodes' |
| 377 | 377 | ); |
| 378 | 378 | |
| 379 | | - // Test 4 |
| 380 | | - // FIXME this test fails |
| | 379 | + // Test 5 |
| 381 | 380 | deepEqual( |
| 382 | 381 | documentModel.prepareRemoval( new es.Range( 3, 24 ) ).getOperations(), |
| 383 | 382 | [ |
| 384 | 383 | { 'type': 'retain', 'length': 3 }, |
| 385 | 384 | { |
| 386 | 385 | 'type': 'remove', |
| 387 | | - 'data': ['c', { 'type': 'textStyle/italic', 'hash': '#textStyle/italic' }] |
| | 386 | + 'data': [['c', { 'type': 'textStyle/italic', 'hash': '#textStyle/italic' }]] |
| 388 | 387 | }, |
| 389 | 388 | { 'type': 'retain', 'length': 4 }, |
| 390 | 389 | { |
| — | — | @@ -406,7 +405,7 @@ |
| 407 | 406 | { 'type': '/listItem' } |
| 408 | 407 | ] |
| 409 | 408 | }, |
| 410 | | - { 'type': 'retain', 'length': 13 } |
| | 409 | + { 'type': 'retain', 'length': 12 } |
| 411 | 410 | ], |
| 412 | 411 | 'prepareRemoval strips and drops correctly when working across structural nodes' |
| 413 | 412 | ); |
| Index: trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js |
| — | — | @@ -823,115 +823,114 @@ |
| 824 | 824 | */ |
| 825 | 825 | es.DocumentModel.prototype.prepareRemoval = function( range ) { |
| 826 | 826 | var doc = this; |
| 827 | | - |
| 828 | | - /** |
| 829 | | - * Return true if can merge the remaining contents of the elements after a selection is deleted |
| 830 | | - * across them. For instance, if a selection is painted across two paragraphs, and then the text |
| 831 | | - * is deleted, the two paragraphs can become one paragraph. However, if the selection crosses |
| 832 | | - * into a table, those cannot be merged. |
| 833 | | - * |
| 834 | | - * @param {Number} integer offset |
| 835 | | - * @param {Number} integer offset |
| 836 | | - * @returns {Boolean} |
| 837 | | - */ |
| 838 | | - function canMerge( range ) { |
| 839 | | - var node1 = doc.getNodeFromOffset( range.start ); |
| 840 | | - var node2 = doc.getNodeFromOffset( range.end ); |
| 841 | | - // This is the simple rule we are following for now -- same type & same parent = can merge. |
| 842 | | - // So you can merge adjacent paragraphs, or listitems. And you can't merge a paragraph into |
| 843 | | - // a table row. There may be other rules we will want in here later, for instance, special |
| 844 | | - // casing merging a listitem into a paragraph. |
| 845 | | - return ( |
| 846 | | - // [<p>a</p><p>b</p>] |
| 847 | | - ( |
| 848 | | - node1 && |
| 849 | | - node2 && |
| 850 | | - node1.getElementType() === node2.getElementType() && |
| 851 | | - node1.getParent() === node2.getParent() |
| 852 | | - ) || |
| 853 | | - // [<p>a</p>]<p>b</p> |
| 854 | | - ( |
| 855 | | - node1 && |
| 856 | | - node2 && |
| 857 | | - node1 === node2 && |
| 858 | | - range.start < range.end |
| 859 | | - ) |
| 860 | | - ); |
| 861 | | - } |
| 862 | 827 | |
| 863 | 828 | /** |
| 864 | | - * Remove all data in a given range. |
| | 829 | + * Remove content only, and completely covered droppable nodes drop the nodes entirely. |
| 865 | 830 | * |
| 866 | | - * @param {es.Range} range Range of data to delete |
| 867 | | - * @param {es.Transaction} tx Transaction to push to |
| | 831 | + * @param {es.DocumentModelNode} node Node to strip from |
| | 832 | + * @param {es.Range} range Range of data to strip |
| 868 | 833 | */ |
| 869 | | - function mergeDelete( range, tx ) { |
| 870 | | - var removed = doc.data.slice( range.start, range.end ); |
| 871 | | - tx.pushRemove( removed ); |
| 872 | | - } |
| 873 | | - |
| 874 | | - /** |
| 875 | | - * Remove content data only, retaining structure |
| 876 | | - * |
| 877 | | - * TODO: Nodes that are completely covered and are droppable should be dropped, not stripped |
| 878 | | - * @see {es.DocumentModel.nodeRules} for obtaining the 'droppable' bit for a given node |
| 879 | | - * |
| 880 | | - * @param {es.Range} range Range of data to delete |
| 881 | | - * @param {es.Transaction} tx Transaction to push to |
| 882 | | - */ |
| 883 | | - function stripDelete( range, tx ) { |
| 884 | | - var lastOperation, operationStart, |
| 885 | | - ops = [], |
| 886 | | - op, |
| 887 | | - i, |
| 888 | | - length; |
| 889 | | - // Get a list of operations, with 0-based indexes |
| 890 | | - for (i = range.start; i < range.end; i++ ) { |
| 891 | | - var neededOp = doc.data[i].type === undefined ? 'remove' : 'retain'; |
| 892 | | - op = ops[ ops.length - 1 ]; |
| 893 | | - if ( op === undefined || op.type !== neededOp ) { |
| 894 | | - ops.push( { type: neededOp, start: i, end: i } ); |
| | 834 | + function strip( tx, node, range, offset ) { |
| | 835 | + var childNodes = node.getChildren(), |
| | 836 | + selectedNodes = node.selectNodes( range ), |
| | 837 | + rules = es.DocumentModel.nodeRules, |
| | 838 | + left = offset || 0, |
| | 839 | + right, |
| | 840 | + elementLength, |
| | 841 | + selectedNode; |
| | 842 | + for ( var i = 0; i < childNodes.length; i++ ) { |
| | 843 | + if ( selectedNodes.length && childNodes[i] === selectedNodes[0].node ) { |
| | 844 | + for ( var j = 0; j < selectedNodes.length; j++ ) { |
| | 845 | + selectedNode = selectedNodes[j]; |
| | 846 | + elementLength = selectedNode.node.getElementLength(); |
| | 847 | + right = left + elementLength; |
| | 848 | + // Handle selected nodes |
| | 849 | + if ( !selectedNode.range ) { |
| | 850 | + // Drop whole nodes |
| | 851 | + if ( rules[selectedNode.node.getElementType()].droppable ) { |
| | 852 | + tx.pushRemove( doc.data.slice( left, right ) ); |
| | 853 | + } else { |
| | 854 | + tx.pushRetain( 1 ); |
| | 855 | + strip( |
| | 856 | + tx, selectedNode.node, new es.Range( 0, elementLength ), left + 1 |
| | 857 | + ); |
| | 858 | + tx.pushRetain( 1 ); |
| | 859 | + } |
| | 860 | + } else { |
| | 861 | + if ( selectedNode.node.hasChildren() ) { |
| | 862 | + tx.pushRetain( 1 ); |
| | 863 | + strip( tx, selectedNode.node, selectedNode.range, left + 1 ); |
| | 864 | + tx.pushRetain( 1 ); |
| | 865 | + } else { |
| | 866 | + // Strip content |
| | 867 | + tx.pushRetain( 1 + selectedNode.range.start ); |
| | 868 | + if ( selectedNode.globalRange.getLength() ) { |
| | 869 | + tx.pushRemove( |
| | 870 | + doc.data.slice( |
| | 871 | + selectedNode.globalRange.start, |
| | 872 | + selectedNode.globalRange.end |
| | 873 | + ) |
| | 874 | + ); |
| | 875 | + } |
| | 876 | + tx.pushRetain( elementLength - selectedNode.range.end - 1 ); |
| | 877 | + } |
| | 878 | + } |
| | 879 | + left = right; |
| | 880 | + } |
| | 881 | + i += selectedNodes.length - 1; |
| 895 | 882 | } else { |
| 896 | | - op.end = i; |
| | 883 | + elementLength = childNodes[i].getElementLength(); |
| | 884 | + right = left + elementLength; |
| | 885 | + // Handle non-selected nodes |
| | 886 | + tx.pushRetain( elementLength ); |
| 897 | 887 | } |
| | 888 | + left = right; |
| 898 | 889 | } |
| 899 | | - // Insert operations as transactions (end must be adjusted) |
| 900 | | - for (i = 0, length = ops.length; i < length; i++ ) { |
| 901 | | - op = ops[i]; |
| 902 | | - if ( op.type === 'retain' ) { |
| 903 | | - // We add one because retain(3,3) really means retain 1 char at pos 3 |
| 904 | | - tx.pushRetain( op.end - op.start + 1 ); |
| 905 | | - } else if ( op.type === 'remove' ) { |
| 906 | | - // We add one because to remove(3,5) we need to slice(3,6), the ending is last |
| 907 | | - // subscript removed + 1. |
| 908 | | - tx.pushRemove( doc.data.slice( op.start, op.end + 1 ) ); |
| 909 | | - } else { |
| 910 | | - throw 'Impossible code path reached.'; |
| 911 | | - } |
| 912 | | - } |
| 913 | 890 | } |
| 914 | 891 | |
| 915 | 892 | var tx = new es.Transaction(); |
| 916 | 893 | range.normalize(); |
| 917 | 894 | |
| 918 | | - // Retain to the start of the range |
| 919 | | - if ( range.start > 0 ) { |
| 920 | | - tx.pushRetain( range.start ); |
| 921 | | - } |
| 922 | | - |
| 923 | | - // Choose a deletion strategy; merging nodes together, or stripping content from existing |
| 924 | | - // structure. |
| 925 | | - if ( canMerge( range ) ) { |
| 926 | | - mergeDelete( range, tx ); |
| | 895 | + var node1 = this.getNodeFromOffset( range.start ); |
| | 896 | + var node2 = this.getNodeFromOffset( range.end ); |
| | 897 | + |
| | 898 | + // If a selection is painted across two paragraphs, and then the text is deleted, the two |
| | 899 | + // paragraphs can become one paragraph. However, if the selection crosses into a table, those |
| | 900 | + // cannot be merged. To make this simple, we are follow a basic rule: |
| | 901 | + // can merge = ( same type ) && ( same parent ) |
| | 902 | + // So you can merge adjacent paragraphs, or listitems. And you can't merge a paragraph into |
| | 903 | + // a table row. There may be other rules we will want in here later, for instance, special |
| | 904 | + // casing merging a listitem into a paragraph. |
| | 905 | + if ( |
| | 906 | + // [<p>a</p><p>b</p>] |
| | 907 | + ( |
| | 908 | + node1 && |
| | 909 | + node2 && |
| | 910 | + node1.getElementType() === node2.getElementType() && |
| | 911 | + node1.getParent() === node2.getParent() |
| | 912 | + ) || |
| | 913 | + // [<p>a</p>]<p>b</p> |
| | 914 | + ( |
| | 915 | + node1 && |
| | 916 | + node2 && |
| | 917 | + node1 === node2 && |
| | 918 | + range.start < range.end |
| | 919 | + ) |
| | 920 | + ) { |
| | 921 | + // Retain to the start of the range |
| | 922 | + if ( range.start > 0 ) { |
| | 923 | + tx.pushRetain( range.start ); |
| | 924 | + } |
| | 925 | + // Remove all data in a given range. |
| | 926 | + tx.pushRemove( this.data.slice( range.start, range.end ) ); |
| | 927 | + // Retain up to the end of the document. Why do we do this? Because Trevor said so! |
| | 928 | + if ( range.end < this.data.length ) { |
| | 929 | + tx.pushRetain( this.data.length - range.end ); |
| | 930 | + } |
| 927 | 931 | } else { |
| 928 | | - stripDelete( range, tx ); |
| | 932 | + strip( tx, this, range ); |
| 929 | 933 | } |
| 930 | 934 | |
| 931 | | - // Retain up to the end of the document. Why do we do this? Because Trevor said so! |
| 932 | | - if ( range.end < doc.data.length ) { |
| 933 | | - tx.pushRetain( doc.data.length - range.end ); |
| 934 | | - } |
| 935 | | - |
| 936 | 935 | tx.optimize(); |
| 937 | 936 | return tx; |
| 938 | 937 | }; |