Fix some failing multitests
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Fri, 22 Jul 2011 10:22:17 -0600
changeset 441 33cf86cf2e6b
parent 440 2452d1c6d2f2
child 442 2c05ce66039c
Fix some failing multitests

Basically related to states/values being restored in the wrong order.
autoimplementation.html
editcommands.html
implementation.js
source.html
tests.css
tests.js
--- a/autoimplementation.html	Thu Jul 21 15:32:46 2011 -0600
+++ b/autoimplementation.html	Fri Jul 22 10:22:17 2011 -0600
@@ -69,6 +69,11 @@
 
 	runTestsButton.parentNode.removeChild(runTestsButton);
 
+	// Make sure styleWithCss is always reset to false at the start of a test
+	// run
+	myExecCommand("stylewithcss", false, "false");
+	try { document.execCommand("stylewithcss", false, "false") } catch(e) {}
+
 	var addTestButton = document.querySelector("#" + command + " button");
 	var inputs = document.getElementById(command).getElementsByTagName("input");
 	for (var i = 0; i < tests[command].length; i++) {
--- a/editcommands.html	Thu Jul 21 15:32:46 2011 -0600
+++ b/editcommands.html	Fri Jul 22 10:22:17 2011 -0600
@@ -855,12 +855,6 @@
 <p class=XXX>Figure out exactly what commands need to preserve state/value
 overrides.
 
-<p>A <dfn id=command-with-insertion-affecting-state>command with insertion-affecting state</dfn> is "bold", "italic",
-"strikethrough", "subscript", "superscript", or "underline".
-
-<p>A <dfn id=command-with-insertion-affecting-value>command with insertion-affecting value</dfn> is "createLink",
-"fontName", "fontSize", "foreColor", or "hiliteColor".
-
 <p>When the user agent is instructed to run a particular method, it must follow
 the steps defined for that method in the appropriate specification, not act as
 though the method had actually been called from JavaScript.  In particular,
@@ -982,47 +976,49 @@
 <p>To <dfn id=record-current-states-and-values>record current states and values</dfn>:
 
 <ol>
-  <li>Let <var title="">states</var> be a dictionary mapping <a href=#command title=command>commands</a> to booleans, initially empty.
-
-  <li>For each <a href=#command-with-insertion-affecting-state>command with insertion-affecting state</a>
-  <var title="">command</var>, in order, if there is a <a href=#state-override>state override</a> for
-  <var title="">command</var>, add an entry to <var title="">states</var> mapping
-  <var title="">command</var> to its <a href=#state-override>state override</a>.
-
-  <li>Let <var title="">values</var> be a dictionary mapping <a href=#command title=command>commands</a> to strings, initially empty.
-
-  <li>For each <a href=#command-with-insertion-affecting-value>command with insertion-affecting value</a>
-  <var title="">command</var>, in order, if there is a <a href=#value-override>value override</a> for
-  <var title="">command</var>, add an entry to <var title="">values</var> mapping
-  <var title="">command</var> to its <a href=#value-override>value override</a>.
-
-  <li>Return (<var title="">states</var>, <var title="">values</var>).
+  <li>Let <var title="">overrides</var> be a list of (string, string or boolean) ordered
+  pairs, initially empty.
+
+  <!--
+  When restoring, some commands can interfere with others.  Specifically, we
+  want to restore createLink before foreColor and underline, and subscript and
+  superscript before fontSize.  TODO: This approach only works for default
+  styles (although I'm not sure offhand how we could handle non-default styles
+  in principle).
+  -->
+  <li>If there is a <a href=#value-override>value override</a> for "createLink", add
+  ("createLink", <a href=#value-override>value override</a> for "createLink") to
+  <var title="">overrides</var>.
+
+  <li>For each <var title="">command</var> in the list "bold", "italic",
+  "strikethrough", "subscript", "superscript", "underline", in order: if there
+  is a <a href=#state-override>state override</a> for <var title="">command</var>, add
+  (<var title="">command</var>, <var title="">command</var>'s <a href=#state-override>state override</a>) to
+  <var title="">overrides</var>.
+
+  <li>For each <var title="">command</var> in the list "fontName", "fontSize",
+  "foreColor", "hiliteColor", in order: if there is a <a href=#value-override>value
+  override</a> for <var title="">command</var>, add (<var title="">command</var>,
+  <var title="">command</var>'s <a href=#value-override>value override</a>) to <var title="">overrides</var>.
+
+  <li>Return <var title="">overrides</var>.
 </ol>
 
-<p>To <dfn id=restore-states-and-values>restore states and values</dfn> specified by dictionaries
-(<var title="">states</var>, <var title="">values</var>) returned by the <a href=#record-current-states-and-values>record current
-states and values</a> algorithm:
+<p>To <dfn id=restore-states-and-values>restore states and values</dfn> specified by a list
+<var title="">overrides</var> returned by the <a href=#record-current-states-and-values>record current states and
+values</a> algorithm:
 
 <ol>
-  <li>For each <var title="">command</var> that is a key in <var title="">states</var>, in order:
+  <li>For each (<var title="">command</var>, <var title="">override</var>) pair in
+  <var title="">overrides</var>, in order:
 
   <ol>
-    <li>Let <var title="">state</var> be the value of <var title="">command</var> in
-    <var title="">states</var>.
-
-    <li>If <code title=queryCommandState()><a href=#querycommandstate()>queryCommandState(<var title="">command</var>)</a></code>
-    returns something different from <var title="">state</var>, call <code title=execCommand()><a href=#execcommand()>execCommand(<var title="">command</var>)</a></code>.
-  </ol>
-
-  <li>For each <var title="">command</var> that is a key in <var title="">values</var>, in order:
-
-  <ol>
-    <li>Let <var title="">value</var> be the value of <var title="">command</var> in
-    <var title="">values</var>.
-
-    <li>If <code title=queryCommandValue()><a href=#querycommandvalue()>queryCommandValue(<var title="">command</var>)</a></code>
-    returns something different from <var title="">value</var>, call <code title=execCommand()><a href=#execcommand()>execCommand(<var title="">command</var>, false,
-    <var title="">value</var>)</a></code>.
+    <li>If <var title="">override</var> is a boolean, and <code title=queryCommandState()><a href=#querycommandstate()>queryCommandState(<var title="">command</var>)</a></code>
+    returns something different from <var title="">override</var>, call <code title=execCommand()><a href=#execcommand()>execCommand(<var title="">command</var>)</a></code>.
+
+    <li>If <var title="">override</var> is a string, and <code title=queryCommandValue()><a href=#querycommandvalue()>queryCommandValue(<var title="">command</var>)</a></code>
+    returns something different from <var title="">override</var>, call <code title=execCommand()><a href=#execcommand()>execCommand(<var title="">command</var>, false,
+    <var title="">override</var>)</a></code>.
   </ol>
 </ol>
 
--- a/implementation.js	Thu Jul 21 15:32:46 2011 -0600
+++ b/implementation.js	Fri Jul 22 10:22:17 2011 -0600
@@ -117,7 +117,7 @@
 
 // Return the <font size=X> value for the given CSS size, or undefined if there
 // is none.
-function getFontSize(cssVal) {
+function cssSizeToLegacy(cssVal) {
 	return {
 		"xx-small": 1,
 		"small": 2,
@@ -134,7 +134,8 @@
 // test implementation.  It's not clear how all this should actually be specced
 // in practice, since CSS defines no notion of equality, does it?
 function valuesEqual(command, val1, val2) {
-	if (commands[command].relevantCssProperty === null) {
+	if (commands[command].relevantCssProperty === null
+	|| (command == "fontsize" && /^[1-7]$/.test(val1) && /^[1-7]$/.test(val2))) {
 		return val1 === val2;
 	}
 
@@ -170,14 +171,14 @@
 	// <font size="1">, and so on.  So we have to test both . . .
 	var test1b = null, test2b = null;
 	if (command == "fontsize") {
-		if (typeof getFontSize(val1) != "undefined") {
+		if (typeof cssSizeToLegacy(val1) != "undefined") {
 			test1b = document.createElement("font");
-			test1b.size = getFontSize(val1);
+			test1b.size = cssSizeToLegacy(val1);
 			document.body.appendChild(test1b);
 		}
-		if (typeof getFontSize(val2) != "undefined") {
+		if (typeof cssSizeToLegacy(val2) != "undefined") {
 			test2b = document.createElement("font");
-			test2b.size = getFontSize(val2);
+			test2b.size = cssSizeToLegacy(val2);
 			document.body.appendChild(test2b);
 		}
 	}
@@ -1052,16 +1053,6 @@
 	}
 })();
 
-// "A command with insertion-affecting state is "bold", "italic",
-// "strikethrough", "subscript", "superscript", or "underline"."
-var commandsWithInsertionAffectingState = ["bold", "italic", "strikethrough",
-	"subscript", "superscript", "underline"];
-
-// "A command with insertion-affecting value is "createLink", "fontName",
-// "fontSize", "forecolor", or "hiliteColor"."
-var commandsWithInsertionAffectingValue = ["createlink", "fontname",
-	"fontsize", "forecolor", "hilitecolor"];
-
 //@}
 
 /////////////////////////////
@@ -1247,61 +1238,59 @@
 }
 
 function recordCurrentStatesAndValues() {
-	// "Let states be a dictionary mapping commands to booleans, initially
-	// empty."
-	var states = {};
-
-	// "For each command with insertion-affecting state command, in order, if
-	// there is a state override for command, add an entry to states mapping
-	// command to its state override."
-	commandsWithInsertionAffectingState.forEach(function(command) {
+	// "Let overrides be a list of (string, string or boolean) ordered pairs,
+	// initially empty."
+	var overrides = [];
+
+	// "If there is a value override for "createLink", add ("createLink", value
+	// override for "createLink") to overrides."
+	if (getValueOverride("createlink") !== undefined) {
+		overrides.push(["createlink", getValueOverride("createlink")]);
+	}
+
+	// "For each command in the list "bold", "italic", "strikethrough",
+	// "subscript", "superscript", "underline", in order: if there is a state
+	// override for command, add (command, command's state override) to
+	// overrides."
+	["bold", "italic", "strikethrough", "subscript", "superscript",
+	"underline"].forEach(function(command) {
 		if (getStateOverride(command) !== undefined) {
-			states[command] = getStateOverride(command);
-		}
-	});
-
-	// "Let values be a dictionary mapping commands to strings, initially
-	// empty."
-	var values = {};
-
-	// "For each command with insertion-affecting value command, in order, if
-	// there is a value override for command, add an entry to values mapping
-	// command to its value override."
-	commandsWithInsertionAffectingValue.forEach(function(command) {
-		if (getValueOverride(command) !== undefined) {
-			values[command] = getValueOverride(command);
+			overrides.push([command, getStateOverride(command)]);
 		}
 	});
 
-	// "Return (states, values)."
-	return [states, values];
+	// "For each command in the list "fontName", "fontSize", "foreColor",
+	// "hiliteColor", in order: if there is a value override for command, add
+	// (command, command's value override) to overrides."
+	["fontname", "fontsize", "forecolor", "hilitecolor"].forEach(function(command) {
+		if (getValueOverride(command) !== undefined) {
+			overrides.push([command, getValueOverride(command)]);
+		}
+	});
+
+	// "Return overrides."
+	return overrides;
 }
 
-function restoreStatesAndValues(record) {
-	var states = record[0];
-	var values = record[1];
-
-	// "For each command that is a key in states, in order:"
-	for (var command in states) {
-		// "Let state be the value of command in states."
-		var state = states[command];
-
-		// "If queryCommandState(command) returns something different from
-		// state, call execCommand(command)."
-		if (myQueryCommandState(command) !== state) {
+function restoreStatesAndValues(overrides) {
+	// "For each (command, override) pair in overrides, in order:"
+	for (var i = 0; i < overrides.length; i++) {
+		var command = overrides[i][0];
+		var override = overrides[i][1];
+
+		// "If override is a boolean, and queryCommandState(command) returns
+		// something different from override, call execCommand(command)."
+		if (typeof override == "boolean"
+		&& myQueryCommandState(command) != override) {
 			myExecCommand(command);
 		}
-	}
-
-	// "For each command that is a key in values, in order:"
-	for (var command in values) {
-		// "Let value be the value of command in values."
-		var value = values[command];
-
-		// "If queryCommandValue(command) returns something different from
-		// value, call execCommand(command, false, value)."
-		if (myQueryCommandValue(command) !== value) {
-			myExecCommand(command, false, value);
+
+		// "If override is a string, and queryCommandValue(command) returns
+		// something different from override, call execCommand(command, false,
+		// override)."
+		if (typeof override == "string"
+		&& !valuesEqual(command, myQueryCommandValue(command), override)) {
+			myExecCommand(command, false, override);
 		}
 	}
 }
--- a/source.html	Thu Jul 21 15:32:46 2011 -0600
+++ b/source.html	Fri Jul 22 10:22:17 2011 -0600
@@ -807,12 +807,6 @@
 <p class=XXX>Figure out exactly what commands need to preserve state/value
 overrides.
 
-<p>A <dfn>command with insertion-affecting state</dfn> is "bold", "italic",
-"strikethrough", "subscript", "superscript", or "underline".
-
-<p>A <dfn>command with insertion-affecting value</dfn> is "createLink",
-"fontName", "fontSize", "foreColor", or "hiliteColor".
-
 <p>When the user agent is instructed to run a particular method, it must follow
 the steps defined for that method in the appropriate specification, not act as
 though the method had actually been called from JavaScript.  In particular,
@@ -936,53 +930,53 @@
 <p>To <dfn>record current states and values</dfn>:
 
 <ol>
-  <li>Let <var>states</var> be a dictionary mapping <span
-  title=command>commands</span> to booleans, initially empty.
-
-  <li>For each <span>command with insertion-affecting state</span>
-  <var>command</var>, in order, if there is a <span>state override</span> for
-  <var>command</var>, add an entry to <var>states</var> mapping
-  <var>command</var> to its <span>state override</span>.
-
-  <li>Let <var>values</var> be a dictionary mapping <span
-  title=command>commands</span> to strings, initially empty.
-
-  <li>For each <span>command with insertion-affecting value</span>
-  <var>command</var>, in order, if there is a <span>value override</span> for
-  <var>command</var>, add an entry to <var>values</var> mapping
-  <var>command</var> to its <span>value override</span>.
-
-  <li>Return (<var>states</var>, <var>values</var>).
+  <li>Let <var>overrides</var> be a list of (string, string or boolean) ordered
+  pairs, initially empty.
+
+  <!--
+  When restoring, some commands can interfere with others.  Specifically, we
+  want to restore createLink before foreColor and underline, and subscript and
+  superscript before fontSize.  TODO: This approach only works for default
+  styles (although I'm not sure offhand how we could handle non-default styles
+  in principle).
+  -->
+  <li>If there is a <span>value override</span> for "createLink", add
+  ("createLink", <span>value override</span> for "createLink") to
+  <var>overrides</var>.
+
+  <li>For each <var>command</var> in the list "bold", "italic",
+  "strikethrough", "subscript", "superscript", "underline", in order: if there
+  is a <span>state override</span> for <var>command</var>, add
+  (<var>command</var>, <var>command</var>'s <span>state override</span>) to
+  <var>overrides</var>.
+
+  <li>For each <var>command</var> in the list "fontName", "fontSize",
+  "foreColor", "hiliteColor", in order: if there is a <span>value
+  override</span> for <var>command</var>, add (<var>command</var>,
+  <var>command</var>'s <span>value override</span>) to <var>overrides</var>.
+
+  <li>Return <var>overrides</var>.
 </ol>
 
-<p>To <dfn>restore states and values</dfn> specified by dictionaries
-(<var>states</var>, <var>values</var>) returned by the <span>record current
-states and values</span> algorithm:
+<p>To <dfn>restore states and values</dfn> specified by a list
+<var>overrides</var> returned by the <span>record current states and
+values</span> algorithm:
 
 <ol>
-  <li>For each <var>command</var> that is a key in <var>states</var>, in order:
+  <li>For each (<var>command</var>, <var>override</var>) pair in
+  <var>overrides</var>, in order:
 
   <ol>
-    <li>Let <var>state</var> be the value of <var>command</var> in
-    <var>states</var>.
-
-    <li>If <code
+    <li>If <var>override</var> is a boolean, and <code
     title=queryCommandState()>queryCommandState(<var>command</var>)</code>
-    returns something different from <var>state</var>, call <code
+    returns something different from <var>override</var>, call <code
     title=execCommand()>execCommand(<var>command</var>)</code>.
-  </ol>
-
-  <li>For each <var>command</var> that is a key in <var>values</var>, in order:
-
-  <ol>
-    <li>Let <var>value</var> be the value of <var>command</var> in
-    <var>values</var>.
-
-    <li>If <code
+
+    <li>If <var>override</var> is a string, and <code
     title=queryCommandValue()>queryCommandValue(<var>command</var>)</code>
-    returns something different from <var>value</var>, call <code
+    returns something different from <var>override</var>, call <code
     title=execCommand()>execCommand(<var>command</var>, false,
-    <var>value</var>)</code>.
+    <var>override</var>)</code>.
   </ol>
 </ol>
 
--- a/tests.css	Thu Jul 21 15:32:46 2011 -0600
+++ b/tests.css	Fri Jul 22 10:22:17 2011 -0600
@@ -95,3 +95,7 @@
 	text-align: center;
 	padding: 2em;
 }
+/* Some tests assume links are blue, for the sake of argument, but they aren't
+ * blue in any browser.  And :visited definitely isn't blue, except in engines
+ * like Gecko that lie. */
+:link, :visited { color: blue }
--- a/tests.js	Thu Jul 21 15:32:46 2011 -0600
+++ b/tests.js	Fri Jul 22 10:22:17 2011 -0600
@@ -791,6 +791,10 @@
 		'foo<span id=purple>ba[r</span>ba]z',
 		'<span style="color: rgb(0, 0, 255)">foo<span id=purple>b[a]r</span>baz</span>',
 
+		['blue', '<a href=http://www.google.com>foo[bar]baz</a>'],
+		['#0000ff', '<a href=http://www.google.com>foo[bar]baz</a>'],
+		['rgb(0,0,255)', '<a href=http://www.google.com>foo[bar]baz</a>'],
+
 		// Tests for queryCommandValue()
 		'<font color="blue">[foo]</font>',
 		'<font color="0000ff">[foo]</font>',
@@ -3307,8 +3311,27 @@
 		['foo[]bar', 'forecolor', ['inserttext', 'a']],
 		['foo[]bar', 'hilitecolor', ['inserttext', 'a']],
 
+		// Test things that interfere with each other
 		['foo[]bar', 'superscript', 'subscript', ['inserttext', 'a']],
 		['foo[]bar', 'subscript', 'superscript', ['inserttext', 'a']],
+
+		['foo[]bar', 'createlink', ['forecolor', '#0000FF'], ['inserttext', 'a']],
+		['foo[]bar', ['forecolor', '#0000FF'], 'createlink', ['inserttext', 'a']],
+		['foo[]bar', 'createlink', ['forecolor', 'blue'], ['inserttext', 'a']],
+		['foo[]bar', ['forecolor', 'blue'], 'createlink', ['inserttext', 'a']],
+		['foo[]bar', 'createlink', ['forecolor', 'brown'], ['inserttext', 'a']],
+		['foo[]bar', ['forecolor', 'brown'], 'createlink', ['inserttext', 'a']],
+		['foo[]bar', 'createlink', ['forecolor', 'black'], ['inserttext', 'a']],
+		['foo[]bar', ['forecolor', 'black'], 'createlink', ['inserttext', 'a']],
+		['foo[]bar', 'createlink', 'underline', ['inserttext', 'a']],
+		['foo[]bar', 'underline', 'createlink', ['inserttext', 'a']],
+		['foo[]bar', 'createlink', 'underline', 'underline', ['inserttext', 'a']],
+		['foo[]bar', 'underline', 'underline', 'createlink', ['inserttext', 'a']],
+
+		['foo[]bar', 'subscript', ['fontsize', '2'], ['inserttext', 'a']],
+		['foo[]bar', ['fontsize', '2'], 'subscript', ['inserttext', 'a']],
+		['foo[]bar', 'subscript', ['fontsize', '3'], ['inserttext', 'a']],
+		['foo[]bar', ['fontsize', '3'], 'subscript', ['inserttext', 'a']],
 	],
 	//@}
 };