r103475 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103474‎ | r103475 | r103476 >
Date:16:44, 17 November 2011
Author:catrope
Status:deferred
Tags:
Comment:
Replace prepareRemoval() with a fixed-up version of my rewrite (which was commented out previously)
Modified paths:
  • /trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js (modified) (history)

Diff [purge]

Index: trunk/extensions/VisualEditor/modules/es/models/es.DocumentModel.js
@@ -821,132 +821,14 @@
822822 * @param {es.Range} range
823823 * @returns {es.Transaction}
824824 */
825 -es.DocumentModel.prototype.prepareRemoval = function( range ) {
826 - var doc = this;
827 -
828 - /**
829 - * Remove content only, and completely covered droppable nodes drop the nodes entirely.
830 - *
831 - * @param {es.DocumentModelNode} node Node to strip from
832 - * @param {es.Range} range Range of data to strip
833 - */
834 - function strip( tx, node, range, offset ) {
835 - var childNodes = node.getChildren(),
836 - selectedNodes = node.selectNodes( range, true ),
837 - rules = es.DocumentModel.nodeRules,
838 - left = offset || 0,
839 - right,
840 - elementLength,
841 - selectedNode;
842 - offset = offset || 0;
843 - for ( var i = 0; i < childNodes.length; i++ ) {
844 - if ( selectedNodes.length && childNodes[i] === selectedNodes[0].node ) {
845 - for ( var j = 0; j < selectedNodes.length; j++ ) {
846 - selectedNode = selectedNodes[j];
847 - elementLength = selectedNode.node.getElementLength();
848 - right = left + elementLength;
849 - // Handle selected nodes
850 - if ( !selectedNode.range ) {
851 - // Drop whole nodes
852 - if ( /*rules[selectedNode.node.getElementType()].droppable*/ true ) {
853 - tx.pushRemove( doc.data.slice( left, right ) );
854 - } else {
855 - tx.pushRetain( 1 );
856 - strip(
857 - tx, selectedNode.node, new es.Range( 0, elementLength ), left + 1
858 - );
859 - tx.pushRetain( 1 );
860 - }
861 - } else {
862 - if ( selectedNode.node.hasChildren() ) {
863 - tx.pushRetain( 1 );
864 - strip( tx, selectedNode.node, selectedNode.range, left + 1 );
865 - tx.pushRetain( 1 );
866 - } else {
867 - // Strip content
868 - tx.pushRetain( 1 + selectedNode.range.start );
869 - if ( selectedNode.globalRange.getLength() ) {
870 - tx.pushRemove(
871 - doc.data.slice(
872 - selectedNode.globalRange.start + offset,
873 - selectedNode.globalRange.end + offset
874 - )
875 - );
876 - }
877 - tx.pushRetain( elementLength - selectedNode.range.end - 1 );
878 - }
879 - }
880 - left = right;
881 - }
882 - i += selectedNodes.length - 1;
883 - } else {
884 - elementLength = childNodes[i].getElementLength();
885 - right = left + elementLength;
886 - // Handle non-selected nodes
887 - tx.pushRetain( elementLength );
888 - }
889 - left = right;
890 - }
891 - }
892825
893 - var tx = new es.Transaction();
894 - range.normalize();
895 -
896 - var node1 = this.getNodeFromOffset( range.start );
897 - var node2 = this.getNodeFromOffset( range.end );
898 -
899 - // If a selection is painted across two paragraphs, and then the text is deleted, the two
900 - // paragraphs can become one paragraph. However, if the selection crosses into a table, those
901 - // cannot be merged. To make this simple, we are follow a basic rule:
902 - // can merge = ( same type ) && ( same parent )
903 - // So you can merge adjacent paragraphs, or listitems. And you can't merge a paragraph into
904 - // a table row. There may be other rules we will want in here later, for instance, special
905 - // casing merging a listitem into a paragraph.
906 - if ( node1 && node2 &&
907 - node1.getElementType() === node2.getElementType() &&
908 - node1.getParent() === node2.getParent()
909 - ) {
910 - // Retain to the start of the range
911 - if ( range.start > 0 ) {
912 - tx.pushRetain( range.start );
913 - }
914 - // Remove all data in a given range.
915 - tx.pushRemove( this.data.slice( range.start, range.end ) );
916 - // Retain up to the end of the document. Why do we do this? Because Trevor said so!
917 - if ( range.end < this.data.length ) {
918 - tx.pushRetain( this.data.length - range.end );
919 - }
920 - } else {
921 - strip( tx, this, range );
922 - }
923 -
924 - tx.optimize();
925 - return tx;
926 -};
927 -
928 -/**
929 - * Generates a transaction which removes data from a given range.
930 - *
931 - * When removing data inside an element, the data is simply discarded and the node's length is
932 - * adjusted accordingly. When removing data across elements, there are two situations that can cause
933 - * added complexity:
934 - * 1. A range spans between nodes of different levels or types
935 - * 2. A range only partially covers one or two nodes
936 - *
937 - * To resolve these issues in a predictable way the following rules must be obeyed:
938 - * 1. Structural elements are retained unless the range being removed covers the entire element
939 - * 2. Elements can only be merged if they are of the same time and share a common parent
940 - *
941 - * @method
942 - * @param {es.Range} range
943 - * @returns {es.Transaction}
944 - */
945 -/*
946 -es.DocumentModel.prototype.prepareRemovalRoan = function( range ) {
 826+es.DocumentModel.prototype.prepareRemoval = function( range ) {
947827 var tx = new es.Transaction(), selectedNodes, selectedNode, startNode, endNode, i;
948828 range.normalize();
949829 if ( range.start === range.end ) {
950 - // Empty range, return empty transaction
 830+ // Empty range, nothing to do
 831+ // Retain up to the end of the document. Why do we do this? Because Trevor said so!
 832+ tx.pushRetain( this.data.length );
951833 return tx;
952834 }
953835
@@ -968,7 +850,6 @@
969851 // This is the simple case. node1 and node2 are either the same node, or can be merged
970852 // So we can just remove all the data in the range and call it a day, no fancy
971853 // processing necessary
972 - // FIXME we're not accounting for droppability here, should we?
973854
974855 // Retain to the start of the range
975856 if ( range.start > 0 ) {
@@ -981,43 +862,35 @@
982863 tx.pushRetain( this.data.length - range.end );
983864 }
984865 } else {
985 - //if ( range.start > 0 ) {
986 - // tx.pushRetain( range.start );
987 - //}
988 -
 866+ var index = 0;
989867 for ( i = 0; i < selectedNodes.length; i++ ) {
990868 selectedNode = selectedNodes[i];
991 - if ( !selectedNode.range ) {
992 - // Remove the entire node
993 - // TODO accounting for droppability will suck
994 - tx.pushRemove( this.data.slice( selectedNode.globalRange.start, selectedNode.globalRange.end ) );
995 - } else {
996 - // Remove part of the node
997 - // TODO account for droppability
998 - // TODO need to descend, rawr
999 - tx.pushRetain( 1 + selectedNode.range.start );
1000 - if ( selectedNode.globalRange.getLength() ) {
1001 - tx.pushRemove(
1002 - this.data.slice(
1003 - selectedNode.globalRange.start,
1004 - selectedNode.globalRange.end
1005 - )
1006 - );
1007 - }
1008 - tx.pushRetain( selectedNode.node.getElementLength() - selectedNode.range.end - 1 );
 869+ // Retain up to where the next removal starts
 870+ if ( selectedNode.globalRange.start > index ) {
 871+ tx.pushRetain( selectedNode.globalRange.start - index );
1009872 }
 873+
 874+ // Remove stuff
 875+ if ( selectedNode.globalRange.getLength() ) {
 876+ tx.pushRemove(
 877+ this.data.slice(
 878+ selectedNode.globalRange.start,
 879+ selectedNode.globalRange.end
 880+ )
 881+ );
 882+ }
 883+ index = selectedNode.globalRange.end;
1010884 }
1011885
1012886 // Retain up to the end of the document. Why do we do this? Because Trevor said so!
1013 - if ( range.end < this.data.length ) {
1014 - tx.pushRetain( this.data.length - range.end );
 887+ if ( index < this.data.length ) {
 888+ tx.pushRetain( this.data.length - index );
1015889 }
1016890 }
1017891
1018892 tx.optimize();
1019893 return tx;
1020894 };
1021 -*/
1022895
1023896 /**
1024897 * Generates a transaction which annotates content within a given range.

Status & tagging log