Fix a subtle bug in restoring overrides
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Wed, 17 Aug 2011 15:15:27 -0600
changeset 537 604b0ea1165a
parent 536 7b22f518099b
child 538 a2d2ada74d4e
Fix a subtle bug in restoring overrides

This was causing the multitest ["foo[]bar", ["fontsize", "3"],
"subscript", "inserttext"] to fail. When doing the inserttext, it first
produced "foo[a]bar", then "foo<sub>[a]</sub>bar". But at this point,
the variable "node" was the text node "foo", not "a", since the node was
split. This led to the fontsize command doing nothing, since the node
was already the correct size.
editing.html
implementation.js
source.html
--- a/editing.html	Wed Aug 17 14:44:17 2011 -0600
+++ b/editing.html	Wed Aug 17 15:15:27 2011 -0600
@@ -4367,19 +4367,31 @@
     <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 <var title="">command</var> is not
-    "fontSize", and <code title=queryCommandValue()><a href=#querycommandvalue()>queryCommandValue(<var title="">command</var>)</a></code>
+    <li>Otherwise, if <var title="">override</var> is a string, and <var title="">command</var>
+    is not "fontSize", and <code title=queryCommandValue()><a href=#querycommandvalue()>queryCommandValue(<var title="">command</var>)</a></code>
     returns something not <a href=#equivalent-values title="equivalent values">equivalent</a> to
     <var title="">override</var>, call <code title=execCommand()><a href=#execcommand()>execCommand(<var title="">command</var>, false,
     <var title="">override</var>)</a></code>.
 
-    <li>If <var title="">override</var> is a string; and <var title="">command</var> is
-    "fontSize"; and either there is a <a href=#value-override>value override</a> for
+    <li>Otherwise, if <var title="">override</var> is a string; and <var title="">command</var>
+    is "fontSize"; and either there is a <a href=#value-override>value override</a> for
     "fontSize" that is not equal to <var title="">override</var>, or there is no
     <a href=#value-override>value override</a> for "fontSize" and <var title="">node</var>'s
     <a href=#effective-command-value>effective command value</a> for "fontSize" is not
     <a href=#loosely-equivalent-values title="loosely equivalent values">loosely equivalent</a> to <var title="">override</var>: call
     <code title=execCommand()><a href=#execcommand()>execCommand("fontSize", false, <var title="">override</var>)</a></code>.
+
+    <li>Otherwise, continue this loop from the beginning.
+
+    <li>
+    <p class=comments>If we called <code title="">execCommand()</code>, we need to reset
+    <var title="">node</var>, because it might have changed.  For instance, if the
+    selection was <code title="">foo[bar]baz</code>, the text node could have been split so
+    that the first part is now outside the active range.
+
+    <p>Set <var title="">node</var> to the first <a href=#editable>editable</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
+    <a href=#effectively-contained>effectively contained</a> in the <a href=#active-range>active range</a>, if
+    there is one.
   </ol>
 
   <li>Otherwise, for each (<var title="">command</var>, <var title="">override</var>) pair in
--- a/implementation.js	Wed Aug 17 14:44:17 2011 -0600
+++ b/implementation.js	Wed Aug 17 15:15:27 2011 -0600
@@ -4185,10 +4185,11 @@
 		}
 	});
 
-	// "For each command in the list "fontName", "foreColor", "hiliteColor", in
-	// order: if there is a value override for command, add (command, command's
-	// value override) to overrides."
-	["fontname", "forecolor", "hilitecolor"].forEach(function(command) {
+	// "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)]);
 		}
@@ -4264,24 +4265,23 @@
 			if (typeof override == "boolean"
 			&& myQueryCommandState(command) != override) {
 				myExecCommand(command);
-			}
-
-			// "If override is a string, and command is not "fontSize", and
-			// queryCommandValue(command) returns something not equivalent to
-			// override, call execCommand(command, false, override)."
-			if (typeof override == "string"
+
+			// "Otherwise, if override is a string, and command is not
+			// "fontSize", and queryCommandValue(command) returns something not
+			// equivalent to override, call execCommand(command, false,
+			// override)."
+			} else if (typeof override == "string"
 			&& command != "fontsize"
 			&& !areEquivalentValues(command, myQueryCommandValue(command), override)) {
 				myExecCommand(command, false, override);
-			}
-
-			// "If override is a string; and command is "fontSize"; and either
-			// there is a value override for "fontSize" that is not equal to
-			// override, or there is no value override for "fontSize" and
-			// node's effective command value for "fontSize" is not loosely
+
+			// "Otherwise, if override is a string; and command is "fontSize";
+			// and either there is a value override for "fontSize" that is not
+			// equal to override, or there is no value override for "fontSize"
+			// and node's effective command value for "fontSize" is not loosely
 			// equivalent to override: call execCommand("fontSize", false,
 			// override)."
-			if (typeof override == "string"
+			} else if (typeof override == "string"
 			&& command == "fontsize"
 			&& (
 				(
@@ -4293,7 +4293,17 @@
 				)
 			)) {
 				myExecCommand("fontsize", false, override);
+
+			// "Otherwise, continue this loop from the beginning."
+			} else {
+				continue;
 			}
+
+			// "Set node to the first editable Text node effectively contained
+			// in the active range, if there is one."
+			node = getAllEffectivelyContainedNodes(getActiveRange())
+				.filter(function(node) { return isEditable(node) && node.nodeType == Node.TEXT_NODE })[0]
+				|| node;
 		}
 
 	// "Otherwise, for each (command, override) pair in overrides, in order:"
--- a/source.html	Wed Aug 17 14:44:17 2011 -0600
+++ b/source.html	Wed Aug 17 15:15:27 2011 -0600
@@ -4388,21 +4388,33 @@
     returns something different from <var>override</var>, call <code
     title=execCommand()>execCommand(<var>command</var>)</code>.
 
-    <li>If <var>override</var> is a string, and <var>command</var> is not
-    "fontSize", and <code
+    <li>Otherwise, if <var>override</var> is a string, and <var>command</var>
+    is not "fontSize", and <code
     title=queryCommandValue()>queryCommandValue(<var>command</var>)</code>
     returns something not <span title="equivalent values">equivalent</span> to
     <var>override</var>, call <code
     title=execCommand()>execCommand(<var>command</var>, false,
     <var>override</var>)</code>.
 
-    <li>If <var>override</var> is a string; and <var>command</var> is
-    "fontSize"; and either there is a <span>value override</span> for
+    <li>Otherwise, if <var>override</var> is a string; and <var>command</var>
+    is "fontSize"; and either there is a <span>value override</span> for
     "fontSize" that is not equal to <var>override</var>, or there is no
     <span>value override</span> for "fontSize" and <var>node</var>'s
     <span>effective command value</span> for "fontSize" is not
     [[looselyequivalent]] to <var>override</var>: call
     [[execcommand|"fontSize", false, <var>override</var>]].
+
+    <li>Otherwise, continue this loop from the beginning.
+
+    <li>
+    <p class=comments>If we called {{code|execCommand()}}, we need to reset
+    <var>node</var>, because it might have changed.  For instance, if the
+    selection was {{code|foo[bar]baz}}, the text node could have been split so
+    that the first part is now outside the active range.
+
+    <p>Set <var>node</var> to the first <span>editable</span> [[text]] node
+    <span>effectively contained</span> in the <span>active range</span>, if
+    there is one.
   </ol>
 
   <li>Otherwise, for each (<var>command</var>, <var>override</var>) pair in