Test multiple addRange()
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Tue, 11 Oct 2011 11:48:10 -0600
changeset 636 0552ef049b40
parent 635 98671dcd5b1f
child 637 731fcde85327
Test multiple addRange()

Also update the spec to explain the choice of behavior.
editing.html
selecttest/Selection-addRange.html
source.html
--- a/editing.html	Wed Oct 05 14:05:27 2011 -0600
+++ b/editing.html	Tue Oct 11 11:48:10 2011 -0600
@@ -65,7 +65,7 @@
 <body class=draft>
 <div class=head id=head>
 <h1>HTML Editing APIs</h1>
-<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-5-october-2011>Work in Progress &mdash; Last Update 5 October 2011</h2>
+<h2 class="no-num no-toc" id=work-in-progress-&mdash;-last-update-11-october-2011>Work in Progress &mdash; Last Update 11 October 2011</h2>
 <dl>
  <dt>Editor
  <dd>Aryeh Gregor &lt;<a href=mailto:[email protected]>[email protected]</a>&gt;
@@ -1183,10 +1183,16 @@
 <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-tree-ancestor title=concept-tree-ancestor>ancestor</a>, it must <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-throw title=concept-throw>throw</a> an "InvalidModificationError".  Otherwise, it must
 set the <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#context-object>context object</a>'s <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-range title=concept-range>range</a> to a reference to (not a copy of)
 <var title="">range</var>. Since <var title="">range</var> is added by reference, subsequent
-calls to <code title=dom-Selection-getRangeAt><a href=#dom-selection-getrangeat>getRangeAt()</a></code> with a
-suitable argument must return the same object, and any changes that a script
-makes to <var title="">range</var> after it is added must be reflected in the
-<a href=#concept-selection title=concept-selection>selection</a>, until <var title="">range</var> is removed from the list.
+calls to <code title=dom-Selection-getRangeAt><a href=#dom-selection-getrangeat>getRangeAt(0)</a></code> must return
+the same object, and any changes that a script makes to <var title="">range</var> after
+it is added must be reflected in the <a href=#concept-selection title=concept-selection>selection</a>, until something else
+removes or replaces the <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#context-object>context object</a>'s <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-range title=concept-range>range</a>.
+
+<p class=comments>Firefox 9.0a2 allows multiple Ranges per Selection, as noted
+elsewhere, so of course this isn't how it behaves.  Chrome 15 dev seems to
+ignore addRange() if there's already a range.  The spec matches IE9, which
+replaces the existing range.  This is likely to behave closest to Firefox, and
+is also more useful than silent failure.
 
 <p>The <dfn id=dom-selection-removerange title=dom-Selection-removeRange><code>removeRange(<var title="">range</var>)</code></dfn>
 method must set the <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#context-object>context object</a>'s <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-range title=concept-range>range</a> to null if its <a class=external data-anolis-spec=dom href=http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#concept-range title=concept-range>range</a> is
--- a/selecttest/Selection-addRange.html	Wed Oct 05 14:05:27 2011 -0600
+++ b/selecttest/Selection-addRange.html	Tue Oct 11 11:48:10 2011 -0600
@@ -6,114 +6,171 @@
 <script>
 "use strict";
 
-// TODO: Test addRange() on a non-empty Selection, once we have a usable spec
-// for that.
+function testAddRange(exception, range, endpoints, qualifier, testName) {
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
 
-function testAddRange(endpoints) {
-	try {
-		selection.removeAllRanges();
-	} catch (e) {
-		throw {message: "Exception thrown during removeAllRanges(): " + e};
-	}
-	try {
-		var range = ownerDocument(endpoints[0]).createRange();
-		range.setStart(endpoints[0], endpoints[1]);
-		range.setEnd(endpoints[2], endpoints[3]);
-	} catch (e) {
-		throw {message: "Exception thrown during Range creation: " + e};
-	}
-
-	assert_equals(range.startContainer, endpoints[0],
-		"Sanity check: the Range we created must have the desired startContainer");
-	assert_equals(range.startOffset, endpoints[1],
-		"Sanity check: the Range we created must have the desired startOffset");
-	assert_equals(range.endContainer, endpoints[2],
-		"Sanity check: the Range we created must have the desired endContainer");
-	assert_equals(range.endOffset, endpoints[3],
-		"Sanity check: the Range we created must have the desired endOffset");
-
-	try {
 		selection.addRange(range);
-	} catch (e) {
-		throw {message: "Exception thrown during addRange(): " + e};
-	}
-
-	assert_equals(range.startContainer, endpoints[0],
-		"addRange() must not modify the startContainer of the Range it's given");
-	assert_equals(range.startOffset, endpoints[1],
-		"addRange() must not modify the startOffset of the Range it's given");
-	assert_equals(range.endContainer, endpoints[2],
-		"addRange() must not modify the endContainer of the Range it's given");
-	assert_equals(range.endOffset, endpoints[3],
-		"addRange() must not modify the endOffset of the Range it's given");
 
-	assert_equals(selection.rangeCount, 1,
-		"After removeAllRanges() and addRange(), rangeCount must be 1");
-	assert_not_equals(selection.getRangeAt(0), null,
-		"After addRange(), getRangeAt(0) must not return null");
-	assert_equals(typeof selection.getRangeAt(0), "object",
-		"After addRange(), getRangeAt(0) must return an object");
+		assert_equals(range.startContainer, endpoints[0],
+			"addRange() must not modify the startContainer of the Range it's given");
+		assert_equals(range.startOffset, endpoints[1],
+			"addRange() must not modify the startOffset of the Range it's given");
+		assert_equals(range.endContainer, endpoints[2],
+			"addRange() must not modify the endContainer of the Range it's given");
+		assert_equals(range.endOffset, endpoints[3],
+			"addRange() must not modify the endOffset of the Range it's given");
+	}, testName + ": " + qualifier + " addRange() must not throw exceptions or modify the range it's given");
 
-	assert_equals(selection.getRangeAt(0).startContainer, range.startContainer,
-		"After addRange(), startContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).startOffset, range.startOffset,
-		"After addRange(), startOffset of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endContainer, range.endContainer,
-		"After addRange(), endContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endOffset, range.endOffset,
-		"After addRange(), endOffset of the Range returned by getRangeAt(0) must match the added Range");
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
 
-	assert_equals(selection.getRangeAt(0), range,
-		"getRangeAt(0) must return the same object we added");
+		assert_equals(selection.rangeCount, 1, "rangeCount must be 1");
+	}, testName + ": " + qualifier + " addRange() must result in rangeCount being 1");
+
+	// From here on out we check selection.getRangeAt(selection.rangeCount - 1)
+	// so as not to double-fail Gecko.
+
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
+		assert_not_equals(selection.rangeCount, 0, "Cannot proceed with tests if rangeCount is 0");
+
+		var newRange = selection.getRangeAt(selection.rangeCount - 1);
+
+		assert_not_equals(newRange, null,
+			"getRangeAt(rangeCount - 1) must not return null");
+		assert_equals(typeof newRange, "object",
+			"getRangeAt(rangeCount - 1) must return an object");
+
+		assert_equals(newRange.startContainer, range.startContainer,
+			"startContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.startOffset, range.startOffset,
+			"startOffset of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endContainer, range.endContainer,
+			"endContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endOffset, range.endOffset,
+			"endOffset of the Selection's last Range must match the added Range");
+	}, testName + ": " + qualifier + " addRange() must result in the selection's last range having the specified endpoints");
+
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
+		assert_not_equals(selection.rangeCount, 0, "Cannot proceed with tests if rangeCount is 0");
+
+		assert_equals(selection.getRangeAt(selection.rangeCount - 1), range,
+			"getRangeAt(rangeCount - 1) must return the same object we added");
+	}, testName + ": " + qualifier + " addRange() must result in the selection's last range being the same object we added");
 
 	// Let's not test many different modifications -- one should be enough.
-	if (range.startContainer == paras[0].firstChild
-	&& range.startOffset == 0
-	&& range.endContainer == paras[0].firstChild
-	&& range.endOffset == 2) {
-		// Just in case . . .
-		range.setStart(paras[0].firstChild, 1);
-	} else {
-		range.setStart(paras[0].firstChild, 0);
-		range.setEnd(paras[0].firstChild, 2);
-	}
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
+		assert_not_equals(selection.rangeCount, 0, "Cannot proceed with tests if rangeCount is 0");
 
-	assert_equals(selection.getRangeAt(0).startContainer, range.startContainer,
-		"After mutating the added Range, startContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).startOffset, range.startOffset,
-		"After mutating the added Range, startOffset of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endContainer, range.endContainer,
-		"After mutating the added Range, endContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endOffset, range.endOffset,
-		"After mutating the added Range, endOffset of the Range returned by getRangeAt(0) must match the added Range");
+		if (range.startContainer == paras[0].firstChild
+		&& range.startOffset == 0
+		&& range.endContainer == paras[0].firstChild
+		&& range.endOffset == 2) {
+			// Just in case . . .
+			range.setStart(paras[0].firstChild, 1);
+		} else {
+			range.setStart(paras[0].firstChild, 0);
+			range.setEnd(paras[0].firstChild, 2);
+		}
+
+		var newRange = selection.getRangeAt(selection.rangeCount - 1);
+
+		assert_equals(newRange.startContainer, range.startContainer,
+			"After mutating the " + qualifier + " added Range, startContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.startOffset, range.startOffset,
+			"After mutating the " + qualifier + " added Range, startOffset of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endContainer, range.endContainer,
+			"After mutating the " + qualifier + " added Range, endContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endOffset, range.endOffset,
+			"After mutating the " + qualifier + " added Range, endOffset of the Selection's last Range must match the added Range");
+	}, testName + ": modifying the " + qualifier + " added range must modify the Selection's last Range");
 
 	// Now test the other way too.
-	if (selection.getRangeAt(0).startContainer == paras[0].firstChild
-	&& selection.getRangeAt(0).startOffset == 4
-	&& selection.getRangeAt(0).endContainer == paras[0].firstChild
-	&& selection.getRangeAt(0).endOffset == 6) {
-		selection.getRangeAt(0).setStart(paras[0].firstChild, 5);
-	} else {
-		selection.getRangeAt(0).setStart(paras[0].firstChild, 4);
-		selection.getRangeAt(0).setStart(paras[0].firstChild, 6);
-	}
+	test(function() {
+		assert_equals(exception, null, "Test setup must not throw exceptions");
+		assert_not_equals(selection.rangeCount, 0, "Cannot proceed with tests if rangeCount is 0");
 
-	assert_equals(selection.getRangeAt(0).startContainer, range.startContainer,
-		"After mutating the result of getRangeAt(0), startContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).startOffset, range.startOffset,
-		"After mutating the result of getRangeAt(0), startOffset of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endContainer, range.endContainer,
-		"After mutating the result of getRangeAt(0), endContainer of the Range returned by getRangeAt(0) must match the added Range");
-	assert_equals(selection.getRangeAt(0).endOffset, range.endOffset,
-		"After mutating the result of getRangeAt(0), endOffset of the Range returned by getRangeAt(0) must match the added Range");
+		var newRange = selection.getRangeAt(selection.rangeCount - 1);
+
+		if (newRange.startContainer == paras[0].firstChild
+		&& newRange.startOffset == 4
+		&& newRange.endContainer == paras[0].firstChild
+		&& newRange.endOffset == 6) {
+			newRange.setStart(paras[0].firstChild, 5);
+		} else {
+			newRange.setStart(paras[0].firstChild, 4);
+			newRange.setStart(paras[0].firstChild, 6);
+		}
+
+		assert_equals(newRange.startContainer, range.startContainer,
+			"After " + qualifier + " addRange(), after mutating the Selection's last Range, startContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.startOffset, range.startOffset,
+			"After " + qualifier + " addRange(), after mutating the Selection's last Range, startOffset of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endContainer, range.endContainer,
+			"After " + qualifier + " addRange(), after mutating the Selection's last Range, endContainer of the Selection's last Range must match the added Range");
+		assert_equals(newRange.endOffset, range.endOffset,
+			"After " + qualifier + " addRange(), after mutating the Selection's last Range, endOffset of the Selection's last Range must match the added Range");
+	}, testName + ": modifying the Selection's last Range must modify the " + qualifier + " added Range");
 }
 
-var tests = [];
+// Do only n evals, not n^2
+var testRangesEvaled = testRanges.map(eval);
+
 for (var i = 0; i < testRanges.length; i++) {
-	tests.push(["Range " + i + " " + testRanges[i], eval(testRanges[i])]);
+	for (var j = 0; j < testRanges.length; j++) {
+		var testName = "Range " + i + " " + testRanges[i]
+			+ " followed by Range " + j + " " + testRanges[j];
+
+		var exception = null;
+		try {
+			selection.removeAllRanges();
+
+			var endpoints1 = testRangesEvaled[i];
+			var range1 = ownerDocument(endpoints1[0]).createRange();
+			range1.setStart(endpoints1[0], endpoints1[1]);
+			range1.setEnd(endpoints1[2], endpoints1[3]);
+
+			if (range1.startContainer !== endpoints1[0]) {
+				throw "Sanity check: the first Range we created must have the desired startContainer";
+			}
+			if (range1.startOffset !== endpoints1[1]) {
+				throw "Sanity check: the first Range we created must have the desired startOffset";
+			}
+			if (range1.endContainer !== endpoints1[2]) {
+				throw "Sanity check: the first Range we created must have the desired endContainer";
+			}
+			if (range1.endOffset !== endpoints1[3]) {
+				throw "Sanity check: the first Range we created must have the desired endOffset";
+			}
+
+			var endpoints2 = testRangesEvaled[j];
+			var range2 = ownerDocument(endpoints2[0]).createRange();
+			range2.setStart(endpoints2[0], endpoints2[1]);
+			range2.setEnd(endpoints2[2], endpoints2[3]);
+
+			if (range2.startContainer !== endpoints2[0]) {
+				throw "Sanity check: the second Range we created must have the desired startContainer";
+			}
+			if (range2.startOffset !== endpoints2[1]) {
+				throw "Sanity check: the second Range we created must have the desired startOffset";
+			}
+			if (range2.endContainer !== endpoints2[2]) {
+				throw "Sanity check: the second Range we created must have the desired endContainer";
+			}
+			if (range2.endOffset !== endpoints2[3]) {
+				throw "Sanity check: the second Range we created must have the desired endOffset";
+			}
+		} catch (e) {
+			exception = e;
+		}
+
+		testAddRange(exception, range1, endpoints1, "first", testName);
+		testAddRange(exception, range2, endpoints2, "second", testName);
+	}
 }
 
-generate_tests(testAddRange, tests);
-
 testDiv.style.display = "none";
 </script>
--- a/source.html	Wed Oct 05 14:05:27 2011 -0600
+++ b/source.html	Tue Oct 11 11:48:10 2011 -0600
@@ -1134,10 +1134,16 @@
 [[ancestor]], it must [[throw]] an "InvalidModificationError".  Otherwise, it must
 set the [[contextobject]]'s [[range]] to a reference to (not a copy of)
 <var>range</var>. Since <var>range</var> is added by reference, subsequent
-calls to <code title=dom-Selection-getRangeAt>getRangeAt()</code> with a
-suitable argument must return the same object, and any changes that a script
-makes to <var>range</var> after it is added must be reflected in the
-[[selection]], until <var>range</var> is removed from the list.
+calls to <code title=dom-Selection-getRangeAt>getRangeAt(0)</code> must return
+the same object, and any changes that a script makes to <var>range</var> after
+it is added must be reflected in the [[selection]], until something else
+removes or replaces the [[contextobject]]'s [[range]].
+
+<p class=comments>Firefox 9.0a2 allows multiple Ranges per Selection, as noted
+elsewhere, so of course this isn't how it behaves.  Chrome 15 dev seems to
+ignore addRange() if there's already a range.  The spec matches IE9, which
+replaces the existing range.  This is likely to behave closest to Firefox, and
+is also more useful than silent failure.
 
 <p>The <dfn
 title=dom-Selection-removeRange><code>removeRange(<var>range</var>)</code></dfn>