Improve invisible node handling for delete
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Mon, 11 Jul 2011 13:08:29 -0600
changeset 393 ebfe6360328c
parent 392 d9c1f90d71d8
child 394 373d18bf5a0e
Improve invisible node handling for delete
editcommands.html
implementation.js
source.html
tests.js
--- a/editcommands.html	Sun Jul 10 13:54:29 2011 -0600
+++ b/editcommands.html	Mon Jul 11 13:08:29 2011 -0600
@@ -38,7 +38,7 @@
 <body class=draft>
 <div class=head id=head>
 <h1>HTML Editing Commands</h1>
-<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-10-july-2011>Work in Progress &mdash; Last Update 10 July 2011</h2>
+<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-11-july-2011>Work in Progress &mdash; Last Update 11 July 2011</h2>
 <dl>
  <dt>Editor
  <dd>Aryeh Gregor &lt;[email protected]&gt;
@@ -5162,12 +5162,20 @@
   <li>Let <var title="">start node</var> equal <var title="">node</var> and let <var title="">start
   offset</var> equal <var title="">offset</var>.
 
-  <li>While <var title="">start offset</var> is zero, set <var title="">start offset</var> to the
-  <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of <var title="">start node</var> and then set <var title="">start node</var> to its
-  <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a>.
-
-  <p class=XXX>The node at index start offset &minus; 1 might be an invisible
-  node.
+  <li>Repeat the following steps:
+
+  <ol>
+    <li>If <var title="">start offset</var> is zero, set <var title="">start offset</var> to the
+    <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of <var title="">start node</var> and then set <var title="">start node</var> to
+    its <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a>.
+
+    <li>Otherwise, if <var title="">start node</var> has an <a href=#editable>editable</a>
+    <a href=#invisible>invisible</a> <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> <var title="">start offset</var>
+    minus one, remove it from <var title="">start node</var> and subtract one from
+    <var title="">start offset</var>.
+
+    <li>Otherwise, break from this loop.
+  </ol>
 
   <!--
   At the beginning of an indented block, outdent it, similar to a list item.
@@ -5309,9 +5317,18 @@
 
   <!-- General block-merging case. -->
   <li>Otherwise, while <var title="">start node</var> has a <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a>
-  <var title="">start offset</var> minus one, set <var title="">start node</var> to that
-  <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a>, then set <var title="">start offset</var> to the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-node-length title=concept-node-length>length</a> of
-  <var title="">start node</var>.
+  <var title="">start offset</var> minus one:
+
+  <ol>
+    <li>If <var title="">start node</var>'s <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> <var title="">start
+    offset</var> minus one is <a href=#editable>editable</a> and <a href=#invisible>invisible</a>,
+    remove it from <var title="">start node</var>, then subtract one from <var title="">start
+    offset</var>.
+
+    <li>Otherwise, set <var title="">start node</var> to its <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a>
+    <var title="">start offset</var> minus one, then set <var title="">start offset</var> to the
+    <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-node-length title=concept-node-length>length</a> of <var title="">start node</var>.
+  </ol>
 
   <li><a href=#delete-the-contents>Delete the contents</a> of the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a>
   (<var title="">start node</var>, <var title="">start offset</var>) and <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-end title=concept-range-end>end</a>
@@ -5673,11 +5690,19 @@
   <li>Let <var title="">end node</var> equal <var title="">node</var> and let <var title="">end
   offset</var> equal <var title="">offset</var>.
 
-  <li>While <var title="">end offset</var> is the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-node-length title=concept-node-length>length</a> of <var title="">end node</var>, set
-  <var title="">end offset</var> to one plus the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of <var title="">end node</var> and
-  then set <var title="">end node</var> to its <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a>.
-
-  <p class=XXX>The node at index end offset might be an invisible node.
+  <li>Repeat the following steps:
+
+  <ol>
+    <li>If <var title="">end offset</var> is the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-node-length title=concept-node-length>length</a> of <var title="">end node</var>, set
+    <var title="">end offset</var> to one plus the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of <var title="">end node</var> and
+    then set <var title="">end node</var> to its <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-parent title=concept-tree-parent>parent</a>.
+
+    <li>Otherwise, if <var title="">end node</var> has a an <a href=#editable>editable</a>
+    <a href=#invisible>invisible</a> <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> <var title="">end offset</var>,
+    remove it from <var title="">end node</var>.
+
+    <li>Otherwise, break from this loop.
+  </ol>
 
   <!-- No special indentation element behavior for forwardDelete. -->
 
@@ -5717,8 +5742,16 @@
   <!-- No special list-item behavior for forwardDelete. -->
 
   <li>While <var title="">end node</var> has a <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> <var title="">end
-  offset</var>, set <var title="">end node</var> to that <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> and set <var title="">end
-  offset</var> to zero.
+  offset</var>:
+
+  <ol>
+    <li>If <var title="">end node</var>'s <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> <var title="">end offset</var>
+    is <a href=#editable>editable</a> and <a href=#invisible>invisible</a>, remove it from
+    <var title="">end node</var>.
+
+    <li>Otherwise, set <var title="">end node</var> to its <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-child title=concept-tree-child>child</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a>
+    <var title="">end offset</var> and set <var title="">end offset</var> to zero.
+  </ol>
 
   <li><a href=#delete-the-contents>Delete the contents</a> of the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a> with <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a>
   (<var title="">node</var>, <var title="">offset</var>) and <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-end title=concept-range-end>end</a> (<var title="">end node</var>,
--- a/implementation.js	Sun Jul 10 13:54:29 2011 -0600
+++ b/implementation.js	Mon Jul 11 13:08:29 2011 -0600
@@ -5368,11 +5368,28 @@
 		var startNode = node;
 		var startOffset = offset;
 
-		// "While start offset is zero, set start offset to the index of start
-		// node and then set start node to its parent."
-		while (startOffset == 0) {
-			startOffset = getNodeIndex(startNode);
-			startNode = startNode.parentNode;
+		// "Repeat the following steps:"
+		while (true) {
+			// "If start offset is zero, set start offset to the index of start
+			// node and then set start node to its parent."
+			if (startOffset == 0) {
+				startOffset = getNodeIndex(startNode);
+				startNode = startNode.parentNode;
+
+			// "Otherwise, if start node has an editable invisible child with
+			// index start offset minus one, remove it from start node and
+			// subtract one from start offset."
+			} else if (0 <= startOffset - 1
+			&& startOffset - 1 < startNode.childNodes.length
+			&& isEditable(startNode.childNodes[startOffset - 1])
+			&& isInvisible(startNode.childNodes[startOffset - 1])) {
+				startNode.removeChild(startNode.childNodes[startOffset - 1]);
+				startOffset--;
+
+			// "Otherwise, break from this loop."
+			} else {
+				break;
+			}
 		}
 
 		// "If offset is zero, and node has an ancestor container that is both
@@ -5503,13 +5520,25 @@
 			offset = 0;
 
 		// "Otherwise, while start node has a child with index start offset
-		// minus one, set start node to that child, then set start offset to
-		// the length of start node."
+		// minus one:"
 		} else {
 			while (0 <= startOffset - 1
 			&& startOffset - 1 < startNode.childNodes.length) {
-				startNode = startNode.childNodes[startOffset - 1];
-				startOffset = getNodeLength(startNode);
+				// "If start node's child with index start offset minus one is
+				// editable and invisible, remove it from start node, then
+				// subtract one from start offset."
+				if (isEditable(startNode.childNodes[startOffset - 1])
+				&& isInvisible(startNode.childNodes[startOffset - 1])) {
+					startNode.removeChild(startNode.childNodes[startOffset - 1]);
+					startOffset--;
+
+				// "Otherwise, set start node to its child with index start
+				// offset minus one, then set start offset to the length of
+				// start node."
+				} else {
+					startNode = startNode.childNodes[startOffset - 1];
+					startOffset = getNodeLength(startNode);
+				}
 			}
 		}
 
@@ -5829,11 +5858,25 @@
 		var endNode = node;
 		var endOffset = offset;
 
-		// "While end offset is the length of end node, set end offset to one
-		// plus the index of end node and then set end node to its parent."
-		while (endOffset == getNodeLength(endNode)) {
-			endOffset = 1 + getNodeIndex(endNode);
-			endNode = endNode.parentNode;
+		// "Repeat the following steps:"
+		while (true) {
+			// "If end offset is the length of end node, set end offset to one
+			// plus the index of end node and then set end node to its parent."
+			if (endOffset == getNodeLength(endNode)) {
+				endOffset = 1 + getNodeIndex(endNode);
+				endNode = endNode.parentNode;
+
+			// "Otherwise, if end node has a an editable invisible child with
+			// index end offset, remove it from end node."
+			} else if (endOffset < endNode.childNodes.length
+			&& isEditable(endNode.childNodes[endOffset])
+			&& isInvisible(endNode.childNodes[endOffset])) {
+				endNode.removeChild(endNode.childNodes[endOffset]);
+
+			// "Otherwise, break from this loop."
+			} else {
+				break;
+			}
 		}
 
 		// "If the child of end node with index end offset minus one is a
@@ -5872,11 +5915,20 @@
 			return;
 		}
 
-		// "While end node has a child with index end offset, set end node to
-		// that child and set end offset to zero."
+		// "While end node has a child with index end offset:"
 		while (endOffset < endNode.childNodes.length) {
-			endNode = endNode.childNodes[endOffset];
-			endOffset = 0;
+			// "If end node's child with index end offset is editable and
+			// invisible, remove it from end node."
+			if (isEditable(endNode.childNodes[endOffset])
+			&& isInvisible(endNode.childNodes[endOffset])) {
+				endNode.removeChild(endNode.childNodes[endOffset]);
+
+			// "Otherwise, set end node to its child with index end offset and
+			// set end offset to zero."
+			} else {
+				endNode = endNode.childNodes[endOffset];
+				endOffset = 0;
+			}
 		}
 
 		// "Delete the contents of the range with start (node, offset) and end
--- a/source.html	Sun Jul 10 13:54:29 2011 -0600
+++ b/source.html	Mon Jul 11 13:08:29 2011 -0600
@@ -5161,12 +5161,20 @@
   <li>Let <var>start node</var> equal <var>node</var> and let <var>start
   offset</var> equal <var>offset</var>.
 
-  <li>While <var>start offset</var> is zero, set <var>start offset</var> to the
-  [[index]] of <var>start node</var> and then set <var>start node</var> to its
-  [[parent]].
-
-  <p class=XXX>The node at index start offset &minus; 1 might be an invisible
-  node.
+  <li>Repeat the following steps:
+
+  <ol>
+    <li>If <var>start offset</var> is zero, set <var>start offset</var> to the
+    [[index]] of <var>start node</var> and then set <var>start node</var> to
+    its [[parent]].
+
+    <li>Otherwise, if <var>start node</var> has an <span>editable</span>
+    <span>invisible</span> [[child]] with [[index]] <var>start offset</var>
+    minus one, remove it from <var>start node</var> and subtract one from
+    <var>start offset</var>.
+
+    <li>Otherwise, break from this loop.
+  </ol>
 
   <!--
   At the beginning of an indented block, outdent it, similar to a list item.
@@ -5308,9 +5316,18 @@
 
   <!-- General block-merging case. -->
   <li>Otherwise, while <var>start node</var> has a [[child]] with [[index]]
-  <var>start offset</var> minus one, set <var>start node</var> to that
-  [[child]], then set <var>start offset</var> to the [[nodelength]] of
-  <var>start node</var>.
+  <var>start offset</var> minus one:
+
+  <ol>
+    <li>If <var>start node</var>'s [[child]] with [[index]] <var>start
+    offset</var> minus one is <span>editable</span> and <span>invisible</span>,
+    remove it from <var>start node</var>, then subtract one from <var>start
+    offset</var>.
+
+    <li>Otherwise, set <var>start node</var> to its [[child]] with [[index]]
+    <var>start offset</var> minus one, then set <var>start offset</var> to the
+    [[nodelength]] of <var>start node</var>.
+  </ol>
 
   <li><span>Delete the contents</span> of the [[range]] with [[rangestart]]
   (<var>start node</var>, <var>start offset</var>) and [[rangeend]]
@@ -5674,11 +5691,19 @@
   <li>Let <var>end node</var> equal <var>node</var> and let <var>end
   offset</var> equal <var>offset</var>.
 
-  <li>While <var>end offset</var> is the [[length]] of <var>end node</var>, set
-  <var>end offset</var> to one plus the [[index]] of <var>end node</var> and
-  then set <var>end node</var> to its [[parent]].
-
-  <p class=XXX>The node at index end offset might be an invisible node.
+  <li>Repeat the following steps:
+
+  <ol>
+    <li>If <var>end offset</var> is the [[length]] of <var>end node</var>, set
+    <var>end offset</var> to one plus the [[index]] of <var>end node</var> and
+    then set <var>end node</var> to its [[parent]].
+
+    <li>Otherwise, if <var>end node</var> has a an <span>editable</span>
+    <span>invisible</span> [[child]] with [[index]] <var>end offset</var>,
+    remove it from <var>end node</var>.
+
+    <li>Otherwise, break from this loop.
+  </ol>
 
   <!-- No special indentation element behavior for forwardDelete. -->
 
@@ -5718,8 +5743,16 @@
   <!-- No special list-item behavior for forwardDelete. -->
 
   <li>While <var>end node</var> has a [[child]] with [[index]] <var>end
-  offset</var>, set <var>end node</var> to that [[child]] and set <var>end
-  offset</var> to zero.
+  offset</var>:
+
+  <ol>
+    <li>If <var>end node</var>'s [[child]] with [[index]] <var>end offset</var>
+    is <span>editable</span> and <span>invisible</span>, remove it from
+    <var>end node</var>.
+
+    <li>Otherwise, set <var>end node</var> to its [[child]] with [[index]]
+    <var>end offset</var> and set <var>end offset</var> to zero.
+  </ol>
 
   <li><span>Delete the contents</span> of the [[range]] with [[rangestart]]
   (<var>node</var>, <var>offset</var>) and [[rangeend]] (<var>end node</var>,
--- a/tests.js	Sun Jul 10 13:54:29 2011 -0600
+++ b/tests.js	Mon Jul 11 13:08:29 2011 -0600
@@ -414,6 +414,24 @@
 		'foo<br><span></span>[]bar',
 		'<span>foo<span></span></span>[]bar',
 		'foo<span></span><span>[]bar</span>',
+		'foo<div><div><p>[]bar</div></div>',
+		'foo<div><div><p><!--abc-->[]bar</div></div>',
+		'foo<div><div><!--abc--><p>[]bar</div></div>',
+		'foo<div><!--abc--><div><p>[]bar</div></div>',
+		'foo<!--abc--><div><div><p>[]bar</div></div>',
+		'<div><div><p>foo</div></div>[]bar',
+		'<div><div><p>foo</div></div><!--abc-->[]bar',
+		'<div><div><p>foo</div><!--abc--></div>[]bar',
+		'<div><div><p>foo</p><!--abc--></div></div>[]bar',
+		'<div><div><p>foo<!--abc--></div></div>[]bar',
+		'<div><div><p>foo</p></div></div><div><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo<!--abc--></p></div></div><div><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p><!--abc--></div></div><div><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p></div><!--abc--></div><div><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p></div></div><!--abc--><div><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p></div></div><div><!--abc--><div><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p></div></div><div><div><!--abc--><div>[]bar</div></div></div>',
+		'<div><div><p>foo</p></div></div><div><div><div><!--abc-->[]bar</div></div></div>',
 
 		// Uncollapsed selection
 		'foo[bar]baz',
@@ -1161,6 +1179,24 @@
 		'foo[]<span></span><br>bar',
 		'<span>foo[]<span></span></span>bar',
 		'foo[]<span></span><span>bar</span>',
+		'foo[]<div><div><p>bar</div></div>',
+		'foo[]<div><div><p><!--abc-->bar</div></div>',
+		'foo[]<div><div><!--abc--><p>bar</div></div>',
+		'foo[]<div><!--abc--><div><p>bar</div></div>',
+		'foo[]<!--abc--><div><div><p>bar</div></div>',
+		'<div><div><p>foo[]</div></div>bar',
+		'<div><div><p>foo[]</div></div><!--abc-->bar',
+		'<div><div><p>foo[]</div><!--abc--></div>bar',
+		'<div><div><p>foo[]</p><!--abc--></div></div>bar',
+		'<div><div><p>foo[]<!--abc--></div></div>bar',
+		'<div><div><p>foo[]</p></div></div><div><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]<!--abc--></p></div></div><div><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p><!--abc--></div></div><div><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p></div><!--abc--></div><div><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p></div></div><!--abc--><div><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p></div></div><div><!--abc--><div><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p></div></div><div><div><!--abc--><div>bar</div></div></div>',
+		'<div><div><p>foo[]</p></div></div><div><div><div><!--abc-->bar</div></div></div>',
 
 		// Uncollapsed selection (should be same as delete command)
 		'foo[bar]baz',