Use collapsed block prop definition more
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Tue, 02 Aug 2011 14:29:11 -0600
changeset 484 eda1d242f5d0
parent 483 2ff138c0c2e8
child 485 37df467690ef
Use collapsed block prop definition more

This makes insertHTML a bit more robust. Also updated tests.js to
reflect the sad fact that yes, there are nodes that aren't Element or
Text.
editing.html
implementation.js
source.html
tests.js
--- a/editing.html	Tue Aug 02 13:51:26 2011 -0600
+++ b/editing.html	Tue Aug 02 14:29:11 2011 -0600
@@ -842,6 +842,11 @@
 the way we want to do things.  If they are, they need some adjustment, like to
 handle collapsed whitespace nodes.
 
+<p class=comments>TODO: Reconsider whether we want to lump invisible nodes in
+here.  If we don't and change the definition, make sure to audit all callers,
+since then a block could have collapsed block prop descendants that aren't
+children.
+
 <p>A <dfn id=collapsed-block-prop>collapsed block prop</dfn> is either a <a href=#collapsed-line-break>collapsed line
 break</a> that is not an <a href=#extraneous-line-break>extraneous line break</a>, or an
 <code class=external data-anolis-spec=domcore><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#element>Element</a></code> that is an <a href=#inline-node>inline node</a> and whose <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>children</a> are all
@@ -6833,7 +6838,7 @@
   <dd>Outputs &lt;blockquote class="webkit-indent-blockquote" style="margin: 0
   0 0 40px; border: none; padding: 0px"&gt; in both modes for both LTR and RTL
   (which is broken for RTL, since it indents only on the left).
-  
+
   <dt>Opera 11.00
   <dd>Outputs &lt;blockquote&gt;, so it indents on both sides and on the
   top/bottom.
@@ -7068,8 +7073,17 @@
   </div>
 
   <p>If the <a href=#active-range>active range</a>'s <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point-node title=concept-boundary-point-node>node</a> is a <a href=#block-node>block
-  node</a> whose sole <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> is a <code class=external data-anolis-spec=html title="the br element"><a href=http://www.whatwg.org/html/#the-br-element>br</a></code>, and its <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point-offset title=concept-boundary-point-offset>offset</a> is 0,
-  remove its <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point-node title=concept-boundary-point-node>node</a>'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> from it.
+  node</a>:
+
+  <ol>
+    <li>Let <var title="">collapsed block props</var> be all <a href=#editable>editable</a>
+    <a href=#collapsed-block-prop>collapsed block prop</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>children</a> of the <a href=#active-range>active
+    range</a>'s <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point-node title=concept-boundary-point-node>node</a> that have <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> greater than or equal to
+    the <a href=#active-range>active range</a>'s <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-boundary-point-offset title=concept-boundary-point-offset>offset</a>.
+
+    <li>For each <var title="">node</var> in <var title="">collapsed block props</var>, remove
+    <var title="">node</var> from 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>.
+  </ol>
 
   <li>
   <p class=comments>We could canonicalize whitespace after inserting the
--- a/implementation.js	Tue Aug 02 13:51:26 2011 -0600
+++ b/implementation.js	Tue Aug 02 14:29:11 2011 -0600
@@ -6588,14 +6588,21 @@
 		// "Let descendants be all descendants of frag."
 		var descendants = getDescendants(frag);
 
-		// "If the active range's start node is a block node whose sole child
-		// is a br, and its start offset is 0, remove its start node's child
-		// from it."
-		if (isBlockNode(getActiveRange().startContainer)
-		&& getActiveRange().startContainer.childNodes.length == 1
-		&& isHtmlElement(getActiveRange().startContainer.firstChild, "br")
-		&& getActiveRange().startOffset == 0) {
-			getActiveRange().startContainer.removeChild(getActiveRange().startContainer.firstChild);
+		// "If the active range's start node is a block node:"
+		if (isBlockNode(getActiveRange().startContainer)) {
+			// "Let collapsed block props be all editable collapsed block prop
+			// children of the active range's start node that have index
+			// greater than or equal to the active range's start offset."
+			//
+			// "For each node in collapsed block props, remove node from its
+			// parent."
+			[].filter.call(getActiveRange().startContainer.childNodes, function(node) {
+				return isEditable(node)
+					&& isCollapsedBlockProp(node)
+					&& getNodeIndex(node) >= getActiveRange().startOffset;
+			}).forEach(function(node) {
+				node.parentNode.removeChild(node);
+			});
 		}
 
 		// "Call insertNode(frag) on the active range."
--- a/source.html	Tue Aug 02 13:51:26 2011 -0600
+++ b/source.html	Tue Aug 02 14:29:11 2011 -0600
@@ -791,6 +791,11 @@
 the way we want to do things.  If they are, they need some adjustment, like to
 handle collapsed whitespace nodes.
 
+<p class=comments>TODO: Reconsider whether we want to lump invisible nodes in
+here.  If we don't and change the definition, make sure to audit all callers,
+since then a block could have collapsed block prop descendants that aren't
+children.
+
 <p>A <dfn>collapsed block prop</dfn> is either a <span>collapsed line
 break</span> that is not an <span>extraneous line break</span>, or an
 [[element]] that is an <span>inline node</span> and whose [[children]] are all
@@ -6870,7 +6875,7 @@
   <dd>Outputs &lt;blockquote class="webkit-indent-blockquote" style="margin: 0
   0 0 40px; border: none; padding: 0px"> in both modes for both LTR and RTL
   (which is broken for RTL, since it indents only on the left).
-  
+
   <dt>Opera 11.00
   <dd>Outputs &lt;blockquote>, so it indents on both sides and on the
   top/bottom.
@@ -7107,8 +7112,17 @@
   </div>
 
   <p>If the <span>active range</span>'s [[startnode]] is a <span>block
-  node</span> whose sole [[child]] is a [[br]], and its [[startoffset]] is 0,
-  remove its [[startnode]]'s [[child]] from it.
+  node</span>:
+
+  <ol>
+    <li>Let <var>collapsed block props</var> be all <span>editable</span>
+    <span>collapsed block prop</span> [[children]] of the <span>active
+    range</span>'s [[startnode]] that have [[index]] greater than or equal to
+    the <span>active range</span>'s [[startoffset]].
+
+    <li>For each <var>node</var> in <var>collapsed block props</var>, remove
+    <var>node</var> from its [[parent]].
+  </ol>
 
   <li>
   <p class=comments>We could canonicalize whitespace after inserting the
--- a/tests.js	Tue Aug 02 13:51:26 2011 -0600
+++ b/tests.js	Tue Aug 02 14:29:11 2011 -0600
@@ -1766,6 +1766,20 @@
 		[' ', '<p>[foo]</p>'],
 		['<span style=display:none></span>', '<p>[foo]</p>'],
 		['<!--abc-->', '<p>[foo]</p>'],
+
+		['abc', '<p>{}<br></p>'],
+		['<!--abc-->', '<p>{}<br></p>'],
+		['abc', '<p><!--foo-->{}<span><br></span><!--bar--></p>'],
+		['<!--abc-->', '<p><!--foo-->{}<span><br></span><!--bar--></p>'],
+		['abc', '<p>{}<span><!--foo--><br><!--bar--></span></p>'],
+		['<!--abc-->', '<p>{}<span><!--foo--><br><!--bar--></span></p>'],
+
+		['abc', '<p><br>{}</p>'],
+		['<!--abc-->', '<p><br>{}</p>'],
+		['abc', '<p><!--foo--><span><br></span>{}<!--bar--></p>'],
+		['<!--abc-->', '<p><!--foo--><span><br></span>{}<!--bar--></p>'],
+		['abc', '<p><span><!--foo--><br><!--bar--></span>{}</p>'],
+		['<!--abc-->', '<p><span><!--foo--><br><!--bar--></span>{}</p>'],
 	],
 	//@}
 	insertimage: [
@@ -4323,14 +4337,14 @@
 //@{
 	// Handle the collapsed case specially, to avoid confusingly getting the
 	// markers backwards in some cases
-	if (range.startContainer.nodeType == Node.TEXT_NODE) {
+	if (range.startContainer.nodeType == Node.TEXT_NODE
+	|| range.startContainer.nodeType == Node.COMMENT_NODE) {
 		if (range.collapsed) {
 			range.startContainer.insertData(range.startOffset, "[]");
 		} else {
 			range.startContainer.insertData(range.startOffset, "[");
 		}
 	} else {
-		// As everyone knows, the only node types are Text and Element.
 		var marker = range.collapsed ? "{}" : "{";
 		if (range.startOffset != range.startContainer.childNodes.length
 		&& range.startContainer.childNodes[range.startOffset].nodeType == Node.TEXT_NODE) {
@@ -4352,7 +4366,8 @@
 	if (range.collapsed) {
 		return;
 	}
-	if (range.endContainer.nodeType == Node.TEXT_NODE) {
+	if (range.endContainer.nodeType == Node.TEXT_NODE
+	|| range.endContainer.nodeType == Node.COMMENT_NODE) {
 		range.endContainer.insertData(range.endOffset, "]");
 	} else {
 		if (range.endOffset != range.endContainer.childNodes.length