Mercurial > hg > editing / changeset
summary | shortlog | changelog | graph | tags | bookmarks | branches | files | changeset
Fix insertHorizontalRule bugs
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Tue, 12 Jul 2011 15:23:52 -0600
changeset 403 2c30e145f765
parent 402 02076f23006c
child 404 69af92f68feb
Fix insertHorizontalRule bugs

Now it won't delete non-editable nodes, and it won't cause trouble when
the selection spans table cells or similar.
editcommands.html
implementation.js
source.html
tests.js 
--- a/editcommands.html	Tue Jul 12 15:07:26 2011 -0600
+++ b/editcommands.html	Tue Jul 12 15:23:52 2011 -0600
@@ -3499,7 +3499,8 @@
 
 <h3 id=deleting-the-contents-of-a-range><span class=secno>8.4 </span>Deleting the contents of a range</h3>
 
-<p>To <dfn id=delete-the-contents>delete the contents</dfn> of a <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a> <var title="">range</var>:
+<p>To <dfn id=delete-the-contents>delete the contents</dfn> of a <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range title=concept-range>range</a> <var title="">range</var>, given a
+<var title="">block merging</var> flag that defaults to true:
 <!-- TODO: Consider what should happen for block merging in corner cases like
 display: inline-table. -->
 
@@ -3758,11 +3759,11 @@
   where one descends from the other.
   -->
 
-  <li>If <var title="">start block</var> or <var title="">end block</var> is null, or <var title="">start
-  block</var> is not <a href=#in-the-same-editing-host>in the same editing host</a> as <var title="">end
-  block</var>, or <var title="">start block</var> and <var title="">end block</var> are the same,
-  set <var title="">range</var>'s <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 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> and then abort
-  these steps.
+  <li>If <var title="">block merging</var> is false, or <var title="">start block</var> or
+  <var title="">end block</var> is null, or <var title="">start block</var> is not <a href=#in-the-same-editing-host>in the
+  same editing host</a> as <var title="">end block</var>, or <var title="">start block</var>
+  and <var title="">end block</var> are the same, set <var title="">range</var>'s <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
+  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> and then abort these steps.
 
   <!--
   We might have added a br to the start/end block in an earlier step.  Now
@@ -6057,18 +6058,28 @@
   <var title="">range</var>'s <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 (<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> of <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-range-end title=concept-range-end>end</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>, 1 + <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a>
   of <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>).
 
-  <li>Run <code class=external data-anolis-spec=domrange title=dom-Range-deleteContents><a href=http://html5.org/specs/dom-range.html#dom-range-deletecontents>deleteContents()</a></code> on <var title="">range</var>.
-
-  <p class=XXX>This might blow up non-contenteditable stuff.
+  <li><a href=#delete-the-contents>Delete the contents</a> of <var title="">range</var>, with <var title="">block
+  merging</var> false.
 
   <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 neither
   <a href=#editable>editable</a> nor an <a href=#editing-host>editing host</a>, abort these steps.
 
+  <!-- We don't want to call insertNode at the start or end of a text node,
+  because that will leave an empty text node. -->
+  <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 <code class=external data-anolis-spec=domcore><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#text>Text</a></code> node 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 zero, 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 (<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> of <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>, <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of
+  <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>).
+
+  <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 <code class=external data-anolis-spec=domcore><a href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#text>Text</a></code> node 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 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 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>, 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 (<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> of
+  <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>, 1 + <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#concept-indexof title=concept-indexof>index</a> of <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>).
+
   <li>Let <var title="">hr</var> be 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("hr")</a></code> on the
   <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#context-object>context object</a>.
 
-  <li>Run <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="">hr</var>)</a></code> on
-  <var title="">range</var>.
+  <li>Run <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="">hr</var>)</a></code> on <var title="">range</var>.
 
   <li><a href=#fix-disallowed-ancestors>Fix disallowed ancestors</a> of <var title="">hr</var>.
   <!--
@@ -6084,9 +6095,9 @@
   <li>Let <var title="">selection</var> be the result of running <code class=external data-anolis-spec=domrange title=dom-Document-getSelection><a href=http://html5.org/specs/dom-range.html#dom-document-getselection>getSelection()</a></code> on the
   <a class=external data-anolis-spec=domrange href=http://html5.org/specs/dom-range.html#context-object>context object</a>.
 
-  <li>Run <code class=external data-anolis-spec=domrange title=dom-Selection-collapse><a href=http://html5.org/specs/dom-range.html#dom-selection-collapse>collapse()</a></code> on <var title="">selection</var>, with
-  first argument equal to the <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> of <var title="">hr</var> and the second
-  argument equal 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="">hr</var>.
+  <li>Run <code class=external data-anolis-spec=domrange title=dom-Selection-collapse><a href=http://html5.org/specs/dom-range.html#dom-selection-collapse>collapse()</a></code> on <var title="">selection</var>, with first argument equal
+  to the <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> of <var title="">hr</var> and the second argument equal 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="">hr</var>.
 </ol>
 
 
--- a/implementation.js	Tue Jul 12 15:07:26 2011 -0600
+++ b/implementation.js	Tue Jul 12 15:23:52 2011 -0600
@@ -3956,17 +3956,32 @@
 ///// Deleting the contents of a range /////
 //@{
 
-function deleteContents(node1, offset1, node2, offset2) {
+function deleteContents() {
+	// We accept several different calling conventions:
+	//
+	// 1) A single argument, which is a range.
+	//
+	// 2) Two arguments, the first being a range and the second the
+	// blockMerging flag.
+	//
+	// 3) Four arguments, the start and end of a range.
+	//
+	// 4) Five arguments, the start and end of a range plus blockMerging.
 	var range;
-
-	// We allow passing four arguments instead of one, in which case they're
-	// the start and end points of the range.
-	if (typeof offset1 == "undefined") {
-		range = node1;
+	var blockMerging = true;
+
+	if (arguments.length < 3) {
+		range = arguments[0];
 	} else {
 		range = document.createRange();
-		range.setStart(node1, offset1);
-		range.setEnd(node2, offset2);
+		range.setStart(arguments[0], arguments[1]);
+		range.setEnd(arguments[2], arguments[3]);
+	}
+	if (arguments.length == 2) {
+		blockMerging = arguments[1];
+	}
+	if (arguments.length == 5) {
+		blockMerging = arguments[4];
 	}
 
 	// "If range is null, abort these steps and do nothing."
@@ -4211,10 +4226,12 @@
 	// "Canonicalize whitespace at range's end."
 	canonicalizeWhitespace(range.endContainer, range.endOffset);
 
-	// "If start block or end block is null, or start block is not in the same
-	// editing host as end block, or start block and end block are the same,
-	// set range's end to its start and then abort these steps."
-	if (!startBlock
+	// "If block merging is false, or start block or end block is null, or
+	// start block is not in the same editing host as end block, or start block
+	// and end block are the same, set range's end to its start and then abort
+	// these steps."
+	if (!blockMerging
+	|| !startBlock
 	|| !endBlock
 	|| !inSameEditingHost(startBlock, endBlock)
 	|| startBlock == endBlock) {
@@ -6224,8 +6241,8 @@
 			range.setEnd(range.endContainer.parentNode, 1 + getNodeIndex(range.endContainer));
 		}
 
-		// "Run deleteContents() on the range."
-		range.deleteContents();
+		// "Delete the contents of range, with block merging false."
+		deleteContents(range, false);
 
 		// "If the active range's start node is neither editable nor an editing
 		// host, abort these steps."
@@ -6234,6 +6251,24 @@
 			return;
 		}
 
+		// "If the active range's start node is a Text node and its start
+		// offset is zero, set the active range's start and end to (parent of
+		// start node, index of start node)."
+		if (getActiveRange().startContainer.nodeType == Node.TEXT_NODE
+		&& getActiveRange().startOffset == 0) {
+			getActiveRange().setStart(getActiveRange().startContainer.parentNode, getNodeIndex(getActiveRange().startContainer));
+			getActiveRange().setEnd(getActiveRange().startContainer.parentNode, getNodeIndex(getActiveRange().startContainer));
+		}
+
+		// "If the active range's start node is a Text node and its start
+		// offset is the length of its start node, set the active range's start
+		// and end to (parent of start node, 1 + index of start node)."
+		if (getActiveRange().startContainer.nodeType == Node.TEXT_NODE
+		&& getActiveRange().startOffset == getNodeLength(getActiveRange().startContainer)) {
+			getActiveRange().setStart(getActiveRange().startContainer.parentNode, 1 + getNodeIndex(getActiveRange().startContainer));
+			getActiveRange().setEnd(getActiveRange().startContainer.parentNode, 1 + getNodeIndex(getActiveRange().startContainer));
+		}
+
 		// "Let hr be the result of calling createElement("hr") on the
 		// context object."
 		var hr = document.createElement("hr");
--- a/source.html	Tue Jul 12 15:07:26 2011 -0600
+++ b/source.html	Tue Jul 12 15:23:52 2011 -0600
@@ -3480,7 +3480,8 @@
 
 <h3>Deleting the contents of a range</h3>
 <!-- @{ -->
-<p>To <dfn>delete the contents</dfn> of a [[range]] <var>range</var>:
+<p>To <dfn>delete the contents</dfn> of a [[range]] <var>range</var>, given a
+<var>block merging</var> flag that defaults to true:
 <!-- TODO: Consider what should happen for block merging in corner cases like
 display: inline-table. -->
 
@@ -3740,11 +3741,11 @@
   where one descends from the other.
   -->
 
-  <li>If <var>start block</var> or <var>end block</var> is null, or <var>start
-  block</var> is not <span>in the same editing host</span> as <var>end
-  block</var>, or <var>start block</var> and <var>end block</var> are the same,
-  set <var>range</var>'s [[rangeend]] to its [[rangestart]] and then abort
-  these steps.
+  <li>If <var>block merging</var> is false, or <var>start block</var> or
+  <var>end block</var> is null, or <var>start block</var> is not <span>in the
+  same editing host</span> as <var>end block</var>, or <var>start block</var>
+  and <var>end block</var> are the same, set <var>range</var>'s [[rangeend]] to
+  its [[rangestart]] and then abort these steps.
 
   <!--
   We might have added a br to the start/end block in an earlier step.  Now
@@ -6052,22 +6053,28 @@
   <var>range</var>'s [[rangeend]] to ([[parent]] of [[endnode]], 1 + [[index]]
   of [[startnode]]).
 
-  <li>Run <code data-anolis-spec=domrange
-  title=dom-Range-deleteContents>deleteContents()</code> on <var>range</var>.
-
-  <p class=XXX>This might blow up non-contenteditable stuff.
+  <li><span>Delete the contents</span> of <var>range</var>, with <var>block
+  merging</var> false.
 
   <li>If the <span>active range</span>'s [[startnode]] is neither
   <span>editable</span> nor an <span>editing host</span>, abort these steps.
 
-  <li>Let <var>hr</var> be the result of calling <code
-  data-anolis-spec=domcore
-  title=dom-Document-createElement>createElement("hr")</code> on the
+  <!-- We don't want to call insertNode at the start or end of a text node,
+  because that will leave an empty text node. -->
+  <li>If the <span>active range</span>'s [[startnode]] is a [[text]] node and
+  its [[startoffset]] is zero, set the <span>active range</span>'s
+  [[rangestart]] and [[rangeend]] to ([[parent]] of [[startnode]], [[index]] of
+  [[startnode]]).
+
+  <li>If the <span>active range</span>'s [[startnode]] is a [[text]] node and
+  its [[startoffset]] is the [[length]] of its [[startnode]], set the
+  <span>active range</span>'s [[rangestart]] and [[rangeend]] to ([[parent]] of
+  [[startnode]], 1 + [[index]] of [[startnode]]).
+
+  <li>Let <var>hr</var> be the result of calling [[createelement|"hr"]] on the
   [[contextobject]].
 
-  <li>Run <code data-anolis-spec=domrange
-  title=dom-Range-insertNode>insertNode(<var>hr</var>)</code> on
-  <var>range</var>.
+  <li>Run [[insertnode|<var>hr</var>]] on <var>range</var>.
 
   <li><span>Fix disallowed ancestors</span> of <var>hr</var>.
   <!--
@@ -6080,15 +6087,12 @@
   identical).
   -->
 
-  <li>Let <var>selection</var> be the result of running <code
-  data-anolis-spec=domrange
-  title=dom-Document-getSelection>getSelection()</code> on the
+  <li>Let <var>selection</var> be the result of running [[getselection]] on the
   [[contextobject]].
 
-  <li>Run <code data-anolis-spec=domrange
-  title=dom-Selection-collapse>collapse()</code> on <var>selection</var>, with
-  first argument equal to the [[parent]] of <var>hr</var> and the second
-  argument equal to one plus the [[index]] of <var>hr</var>.
+  <li>Run [[selcollapse|]] on <var>selection</var>, with first argument equal
+  to the [[parent]] of <var>hr</var> and the second argument equal to one plus
+  the [[index]] of <var>hr</var>.
 </ol>
 <!-- @} -->
 
--- a/tests.js	Tue Jul 12 15:07:26 2011 -0600
+++ b/tests.js	Tue Jul 12 15:23:52 2011 -0600
@@ -1647,6 +1647,9 @@
 		'<xmp>foo[bar]baz</xmp>',
 
 		'<quasit>foo[bar]baz</quasit>',
+
+		'<table><tr><td>fo[o<td>b]ar</table>',
+		'fo[o<span contenteditable=false>bar</span>b]az',
 	],
 	//@}
 	inserthtml: [
editing

Hosted by W3C Systems Team, please report bugs to sysreq@w3.org