Fix an insertParagraph bug
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Tue, 14 Jun 2011 12:45:15 -0600
changeset 264 409c5c207d8b
parent 263 f49fb6022d09
child 265 d0c3ee5a4708
Fix an insertParagraph bug

Markup like "<b>foo[]bar</b>baz" was becoming
"<b><p>foo</p><p>[]bar</p></b>baz". Even if we don't care about nesting
a p inside a b, this breaks one line into three instead of two.
editcommands.html
implementation.js
source.html
tests.js
--- a/editcommands.html	Sun Jun 12 15:13:50 2011 -0600
+++ b/editcommands.html	Tue Jun 14 12:45:15 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-12-june-2011>Work in Progress &mdash; Last Update 12 June 2011</h2>
+<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-14-june-2011>Work in Progress &mdash; Last Update 14 June 2011</h2>
 <dl>
  <dt>Editor
  <dd>Aryeh Gregor &lt;[email protected]&gt;
@@ -5022,14 +5022,17 @@
   <!-- Add the default wrapper. -->
 
   <ol>
+    <li>Let <var title="">tag</var> be the <a href=#default-single-line-container-name>default single-line container
+    name</a>.
+
     <li><a href=#block-extend>Block-extend</a> <var title="">range</var>, and let <var title="">new
     range</var> be the result.
 
-    <li>Let <var title="">node list</var> be a list of all <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
-    <var title="">node</var> that are  <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#contained>contained</a> in <var title="">new range</var>.
-
-    <li>Let <var title="">tag</var> be the <a href=#default-single-line-container-name>default single-line container
-    name</a>.
+    <li>Let <var title="">node list</var> be a list of <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-node title=concept-node>nodes</a>, initially empty.
+
+    <li>Append to <var title="">node list</var> the first <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-node title=concept-node>node</a> in <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#tree-order>tree order</a> that
+    is <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#contained>contained</a> in <var title="">new range</var> and is an <a href=#allowed-child>allowed
+    child</a> of "p", if any.
 
     <li>If <var title="">node list</var> is empty:
 
@@ -5048,6 +5051,14 @@
       <li>Abort these steps.
     </ol>
 
+    <li>While the <code class=external data-anolis-spec=domcore title=dom-Node-nextSibling><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-nextsibling>nextSibling</a></code> of the last member of <var title="">node list</var> is
+    not null and is an <a href=#allowed-child>allowed child</a> of "p", append it to
+    <var title="">node list</var>.
+
+    <p class=XXX>It is not at all obvious that this is the correct list of
+    nodes in all cases.  It should probably work because of how the
+    block-extend algorithm works, but further thought would be good.
+
     <li><a href=#wrap>Wrap</a> <var title="">node list</var>, with <a href=#sibling-criteria>sibling
     criteria</a> matching nothing and <a href=#new-parent-instructions>new parent instructions</a>
     returning the result of calling <code class=external data-anolis-spec=domcore title=dom-Document-createElement><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-document-createelement>createElement(<var title="">tag</var>)</a></code> on the
--- a/implementation.js	Sun Jun 12 15:13:50 2011 -0600
+++ b/implementation.js	Tue Jun 14 12:45:15 2011 -0600
@@ -3868,15 +3868,18 @@
 		if (!isEditable(container)
 		|| !inSameEditingHost(container, node)
 		|| !isSingleLineContainer(container)) {
+			// "Let tag be the default single-line container name."
+			var tag = defaultSingleLineContainerName;
+
 			// "Block-extend range, and let new range be the result."
 			var newRange = blockExtendRange(range);
 
-			// "Let node list be a list of all children of node that are
-			// contained in new range."
-			var nodeList = [].filter.call(node.childNodes, function(child) { return isContained(child, newRange) });
-
-			// "Let tag be the default single-line container name."
-			var tag = defaultSingleLineContainerName;
+			// "Let node list be a list of nodes, initially empty."
+			//
+			// "Append to node list the first node in tree order that is
+			// contained in new range and is an allowed child of "p", if any."
+			var nodeList = collectContainedNodes(newRange, function(node) { return isAllowedChild(node, "p") })
+				.slice(0, 1);
 
 			// "If node list is empty:"
 			if (!nodeList.length) {
@@ -3899,6 +3902,13 @@
 				return;
 			}
 
+			// "While the nextSibling of the last member of node list is not
+			// null and is an allowed child of "p", append it to node list."
+			while (nodeList[nodeList.length - 1].nextSibling
+			&& isAllowedChild(nodeList[nodeList.length - 1].nextSibling, "p")) {
+				nodeList.push(nodeList[nodeList.length - 1].nextSibling);
+			}
+
 			// "Wrap node list, with sibling criteria matching nothing and new
 			// parent instructions returning the result of calling
 			// createElement(tag) on the context object.  Set container to the
--- a/source.html	Sun Jun 12 15:13:50 2011 -0600
+++ b/source.html	Tue Jun 14 12:45:15 2011 -0600
@@ -5042,14 +5042,17 @@
   <!-- Add the default wrapper. -->
 
   <ol>
+    <li>Let <var>tag</var> be the <span>default single-line container
+    name</span>.
+
     <li><span>Block-extend</span> <var>range</var>, and let <var>new
     range</var> be the result.
 
-    <li>Let <var>node list</var> be a list of all [[children]] of
-    <var>node</var> that are  [[contained]] in <var>new range</var>.
-
-    <li>Let <var>tag</var> be the <span>default single-line container
-    name</span>.
+    <li>Let <var>node list</var> be a list of [[nodes]], initially empty.
+
+    <li>Append to <var>node list</var> the first [[node]] in [[treeorder]] that
+    is [[contained]] in <var>new range</var> and is an <span>allowed
+    child</span> of "p", if any.
 
     <li>If <var>node list</var> is empty:
 
@@ -5068,6 +5071,14 @@
       <li>Abort these steps.
     </ol>
 
+    <li>While the [[nextsibling]] of the last member of <var>node list</var> is
+    not null and is an <span>allowed child</span> of "p", append it to
+    <var>node list</var>.
+
+    <p class=XXX>It is not at all obvious that this is the correct list of
+    nodes in all cases.  It should probably work because of how the
+    block-extend algorithm works, but further thought would be good.
+
     <li><span>Wrap</span> <var>node list</var>, with <span>sibling
     criteria</span> matching nothing and <span>new parent instructions</span>
     returning the result of calling [[createelement|<var>tag</var>]] on the
--- a/tests.js	Sun Jun 12 15:13:50 2011 -0600
+++ b/tests.js	Tue Jun 14 12:45:15 2011 -0600
@@ -1357,6 +1357,22 @@
 		'<h1>foo</h1>{}<br><h2>bar</h2>',
 		'<p>foo</p><h1>[bar]</h1><p>baz</p>',
 		'<p>foo</p>{<h1>bar</h1>}<p>baz</p>',
+
+		'<table><tr><td>foo[]bar</table>',
+		'<table><tr><td><p>foo[]bar</table>',
+
+		'<span>foo[]bar</span>',
+		'<span>foo[]bar</span>baz',
+		'<b>foo[]bar</b>',
+		'<b>foo[]bar</b>baz',
+		'<b>foo[]</b>bar',
+		'<b>foo[]</b><i>bar</i>',
+		'<b id=x class=y>foo[]bar</b>',
+		'<i><b>foo[]bar</b>baz</i>',
+
+		'<p><b>foo[]bar</b></p>',
+		'<p><b id=x class=y>foo[]bar</b></p>',
+		'<div><b>foo[]bar</b></div>'
 	],
 	insertunorderedlist: [
 		'foo[]bar',