Fix a couple of insertHTML bugs
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Wed, 15 Jun 2011 12:55:55 -0600
changeset 272 e6efe73e13c5
parent 271 63a2f1a1378e
child 273 e4e477a8b88a
Fix a couple of insertHTML bugs
editcommands.html
implementation.js
source.html
tests.js
--- a/editcommands.html	Wed Jun 15 12:33:24 2011 -0600
+++ b/editcommands.html	Wed Jun 15 12:55:55 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-14-june-2011>Work in Progress &mdash; Last Update 14 June 2011</h2>
+<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-15-june-2011>Work in Progress &mdash; Last Update 15 June 2011</h2>
 <dl>
  <dt>Editor
  <dd>Aryeh Gregor &lt;[email protected]&gt;
@@ -616,6 +616,7 @@
     <tr><td>table <td>caption, col, colgroup, tbody, td, tfoot, th, thead, tr
     <tr><td>tbody, tfoot, thead <td>td, th, tr
     <tr><td>tr <td>td, th
+    <tr><td>ol, ul <td>li, ol, ul
   </table>
 
   <li>If <var title="">child</var> is "body", "caption", "col", "colgroup", "frame",
@@ -2769,15 +2770,29 @@
   <li>Let <var title="">frag</var> be the result of calling <code class=external data-anolis-spec=domps title=dom-Range-createContextualFragment><a href=http://html5.org/specs/dom-parsing.html#dom-range-createcontextualfragment>createContextualFragment(<var title="">value</var>)</a></code>
   on the <a href=#active-range>active range</a>.
 
+  <li>Let <var title="">last child</var> be the <code class=external data-anolis-spec=domcore title=dom-Node-lastChild><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-lastchild>lastChild</a></code> of <var title="">frag</var>.
+
+  <li>If <var title="">last child</var> is null, abort these steps.
+  <!-- Firefox 5.0a2 also seems to not add empty elements like <b></b>, but
+  Chrome 13 dev and Opera 11.11 do. -->
+
   <li>Let <var title="">descendants</var> be all <a class=external data-anolis-spec=domcore href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-descendant title=concept-tree-descendant>descendants</a> of <var title="">frag</var>.
 
-  <li>Let <var title="">last child</var> be the <code class=external data-anolis-spec=domcore title=dom-Node-lastChild><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-node-lastchild>lastChild</a></code> of <var title="">frag</var>.
+  <li>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=#prohibited-paragraph-child>prohibited
+  paragraph child</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.
+  <!--
+  This is so we don't get something like
+    <div>[foo]</div>
+    -> <div>{}<br></div>
+    -> <div><p>Some HTML{}</p><br></div>
+  with an extra bogus line break at the end.
+  -->
 
   <li>Call <code class=external data-anolis-spec=domrange title=dom-Range-insertNode><a href=http://html5.org/specs/dom-range.html#dom-range-insertnode>insertNode(<var title="">frag</var>)</a></code> on the <a href=#active-range>active range</a>.
 
-  <li>If <var title="">last child</var> is not null, set the <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-start title=concept-range-start>start</a> 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> of the <a href=#active-range>active range</a> to (<var title="">last child</var>,
-  <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="">last child</var>).
+  <li>Set 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> 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> to
+  (<var title="">last child</var>, <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="">last child</var>).
   <!-- Need to do this before fixing disallowed ancestors, since otherwise the
   last child might have been removed (e.g., it's an li). -->
 
@@ -5465,4 +5480,4 @@
 </ul>
 
 <script src=http://www.whatwg.org/specs/web-apps/current-work/dfn.js></script>
-<!-- vim: set expandtab shiftwidth=2 tabstop=2 [email protected]{,@} foldmethod=marker: -->
+<!-- vim: set expandtab shiftwidth=2 tabstop=2 [email protected]{,@} foldmethod=marker indentexpr=: -->
--- a/implementation.js	Wed Jun 15 12:33:24 2011 -0600
+++ b/implementation.js	Wed Jun 15 12:55:55 2011 -0600
@@ -801,6 +801,9 @@
 			return ["td", "th", "tr"].indexOf(child) != -1;
 		case "tr":
 			return ["td", "th"].indexOf(child) != -1;
+		case "ol":
+		case "ul":
+			return ["li", "ol", "ul"].indexOf(child) != -1;
 	}
 
 	// "If child is "body", "caption", "col", "colgroup", "frame", "frameset",
@@ -3235,21 +3238,34 @@
 		// on the active range."
 		var frag = getActiveRange().createContextualFragment(value);
 
+		// "Let last child be the lastChild of frag."
+		var lastChild = frag.lastChild;
+
+		// "If last child is null, abort these steps."
+		if (!lastChild) {
+			return;
+		}
+
 		// "Let descendants be all descendants of frag."
 		var descendants = getDescendants(frag);
 
-		// "Let last child be the lastChild of frag."
-		var lastChild = frag.lastChild;
+		// "If the active range's start node is a prohibited paragraph child
+		// whose sole child is a br, and its start offset is 0, remove its
+		// start node's child from it."
+		if (isProhibitedParagraphChild(getActiveRange().startContainer)
+		&& getActiveRange().startContainer.childNodes.length == 1
+		&& isHtmlElement(getActiveRange().startContainer.firstChild, "br")
+		&& getActiveRange().startOffset == 0) {
+			getActiveRange().startContainer.removeChild(getActiveRange().startContainer.firstChild);
+		}
 
 		// "Call insertNode(frag) on the active range."
 		getActiveRange().insertNode(frag);
 
-		// "If last child is not null, set the start and end of the active
-		// range to (last child, length of last child)."
-		if (lastChild) {
-			getActiveRange().setStart(lastChild, getNodeLength(lastChild));
-			getActiveRange().setEnd(lastChild, getNodeLength(lastChild));
-		}
+		// "Set the active range's start and end to (last child, length of last
+		// child)."
+		getActiveRange().setStart(lastChild, getNodeLength(lastChild));
+		getActiveRange().setEnd(lastChild, getNodeLength(lastChild));
 
 		// "Fix disallowed ancestors of each member of descendants."
 		for (var i = 0; i < descendants.length; i++) {
--- a/source.html	Wed Jun 15 12:33:24 2011 -0600
+++ b/source.html	Wed Jun 15 12:55:55 2011 -0600
@@ -562,6 +562,7 @@
     <tr><td>table <td>caption, col, colgroup, tbody, td, tfoot, th, thead, tr
     <tr><td>tbody, tfoot, thead <td>td, th, tr
     <tr><td>tr <td>td, th
+    <tr><td>ol, ul <td>li, ol, ul
   </table>
 
   <li>If <var>child</var> is "body", "caption", "col", "colgroup", "frame",
@@ -2750,15 +2751,29 @@
   title=dom-Range-createContextualFragment>createContextualFragment(<var>value</var>)</code>
   on the <span>active range</span>.
 
+  <li>Let <var>last child</var> be the [[lastchild]] of <var>frag</var>.
+
+  <li>If <var>last child</var> is null, abort these steps.
+  <!-- Firefox 5.0a2 also seems to not add empty elements like <b></b>, but
+  Chrome 13 dev and Opera 11.11 do. -->
+
   <li>Let <var>descendants</var> be all [[descendants]] of <var>frag</var>.
 
-  <li>Let <var>last child</var> be the [[lastchild]] of <var>frag</var>.
+  <li>If the <span>active range</span>'s [[startnode]] is a <span>prohibited
+  paragraph child</span> whose sole [[child]] is a [[br]], and its
+  [[startoffset]] is 0, remove its [[startnode]]'s [[child]] from it.
+  <!--
+  This is so we don't get something like
+    <div>[foo]</div>
+    -> <div>{}<br></div>
+    -> <div><p>Some HTML{}</p><br></div>
+  with an extra bogus line break at the end.
+  -->
 
   <li>Call [[insertnode|<var>frag</var>]] on the <span>active range</span>.
 
-  <li>If <var>last child</var> is not null, set the [[rangestart]] and
-  [[rangeend]] of the <span>active range</span> to (<var>last child</var>,
-  [[nodelength]] of <var>last child</var>).
+  <li>Set the <span>active range</span>'s [[rangestart]] and [[rangeend]] to
+  (<var>last child</var>, [[nodelength]] of <var>last child</var>).
   <!-- Need to do this before fixing disallowed ancestors, since otherwise the
   last child might have been removed (e.g., it's an li). -->
 
@@ -5498,4 +5513,4 @@
 </ul>
 
 <script src=http://www.whatwg.org/specs/web-apps/current-work/dfn.js></script>
-<!-- vim: set expandtab shiftwidth=2 tabstop=2 [email protected]{,@} foldmethod=marker: -->
+<!-- vim: set expandtab shiftwidth=2 tabstop=2 [email protected]{,@} foldmethod=marker indentexpr=: -->
--- a/tests.js	Wed Jun 15 12:33:24 2011 -0600
+++ b/tests.js	Wed Jun 15 12:55:55 2011 -0600
@@ -1057,6 +1057,8 @@
 		['<p>abc', '<p>foo[bar]baz'],
 		['<li>abc', '<p>foo[bar]baz'],
 		['<p>abc', '<ol>{<li>foo</li>}<li>bar</ol>'],
+		['<p>abc', '<ol><li>foo</li>{<li>bar</li>}<li>baz</ol>'],
+		['<p>abc', '<ol><li>[foo]</li><li>bar</ol>'],
 	],
 	insertimage: [
 		'foo[]bar',