Get fontSize working right
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Thu, 14 Jul 2011 14:21:49 -0600
changeset 427 0de6ed26529c
parent 426 66e76e5f2d15
child 428 306a0a84a3fc
Get fontSize working right
editcommands.html
implementation.js
source.html
tests.js
--- a/editcommands.html	Thu Jul 14 13:18:24 2011 -0600
+++ b/editcommands.html	Thu Jul 14 14:21:49 2011 -0600
@@ -2008,8 +2008,8 @@
     opaque with red, green, and blue components in the range 0 to 255:
     <!-- See comment for foreColor for discussion. -->
 
-    <p class=XXX>Check this more carefully for what happens if the components
-    are not integers or are out-of-range.
+    <!-- TODO: Define more carefully what happens when things are out of range
+    or not integers or whatever. -->
 
     <ol>
       <li>Let <var title="">new parent</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("font")</a></code> on the
@@ -2269,12 +2269,15 @@
       <dt>underline<dd>True if <var title="">new value</var> is "underline", false otherwise.
     </dl>
 
-    <li>If <var title="">command</var> has a <a href=#value>value</a> specified, unset the
-    <a href=#value-override>value override</a> if <var title="">new value</var> is null, and set the
-    <a href=#value-override>value override</a> to <var title="">new value</var> if it is not null.
-
-    <p class=XXX>This doesn't work as-is for fontSize, because that uses a
-    different format for the value.
+    <li>If <var title="">new value</var> is null, unset the <a href=#value-override>value override</a>
+    (if any).
+
+    <li>Otherwise, if <var title="">command</var> is "fontSize", set the <a href=#value-override>value
+    override</a> to the <a href=#legacy-font-size-for>legacy font size for</a> the result of
+    converting <var title="">new value</var> to pixels.
+
+    <li>Otherwise, if <var title="">command</var> has a <a href=#value>value</a> specified,
+    set the <a href=#value-override>value override</a> to <var title="">new value</var>.
 
     <li>Abort these steps.
   </ol>
@@ -2741,6 +2744,15 @@
   cannot be null, since the boundary point node of a selection must always be
   either an element or a text node that's the child of an element.
 
+  <li>Return the <a href=#legacy-font-size-for>legacy font size for</a> <var title="">pixel size</var>.
+</ol>
+
+<p><a href=#relevant-css-property>Relevant CSS property</a>: "font-size"
+
+<p>The <dfn id=legacy-font-size-for>legacy font size for</dfn> an integer <var title="">pixel size</var> is
+returned by the following algorithm:
+
+<ol>
   <li>Let <var title="">returned size</var> be 1.
 
   <li>While <var title="">returned size</var> is less than 7:
@@ -2766,9 +2778,6 @@
   <li>Return "7".
 </ol>
 
-<p><a href=#relevant-css-property>Relevant CSS property</a>: "font-size"
-
-
 <h3 id=the-forecolor-command><span class=secno>7.12 </span><dfn>The <code title="">foreColor</code> command</dfn></h3>
 
 <p><a href=#action>Action</a>:
--- a/implementation.js	Thu Jul 14 13:18:24 2011 -0600
+++ b/implementation.js	Thu Jul 14 14:21:49 2011 -0600
@@ -2853,15 +2853,27 @@
 			case "underline": setStateOverride(command, newValue === "underline");
 		}
 
-		// "If command has a value specified, unset the value override if new
-		// value is null, and set the value override to new value if it is not
-		// null."
-		if ("value" in commands[command]) {
-			if (newValue === null) {
-				unsetValueOverride(command);
+		// "If new value is null, unset the value override (if any)."
+		if (newValue === null) {
+			unsetValueOverride(command);
+
+		// "Otherwise, if command is "fontSize", set the value override to the
+		// legacy font size for the result of converting new value to pixels."
+		} else if (command == "fontsize") {
+			var font = document.createElement("font");
+			document.body.appendChild(font);
+			if (newValue == "xxx-large") {
+				font.size = 7;
 			} else {
-				setValueOverride(command, newValue);
+				font.style.fontSize = newValue;
 			}
+			setValueOverride(command, getLegacyFontSize(parseInt(getComputedStyle(font).fontSize)));
+			document.body.removeChild(font);
+
+		// "Otherwise, if command has a value specified, set the value override
+		// to new value."
+		} else if ("value" in commands[command]) {
+			setValueOverride(command, newValue);
 		}
 
 		// "Abort these steps."
@@ -3091,6 +3103,79 @@
 //@}
 ///// The fontSize command /////
 //@{
+
+// Helper function for fontSize's action plus queryOutputHelper.  It's just the
+// middle of fontSize's action, ripped out into its own function.
+function normalizeFontSize(value) {
+	// "Strip leading and trailing whitespace from value."
+	//
+	// Cheap hack, not following the actual algorithm.
+	value = value.trim();
+
+	// "If value is a valid floating point number, or would be a valid
+	// floating point number if a single leading "+" character were
+	// stripped:"
+	if (/^[-+]?[0-9]+(\.[0-9]+)?([eE][-+]?[0-9]+)?$/.test(value)) {
+		var mode;
+
+		// "If the first character of value is "+", delete the character
+		// and let mode be "relative-plus"."
+		if (value[0] == "+") {
+			value = value.slice(1);
+			mode = "relative-plus";
+		// "Otherwise, if the first character of value is "-", delete the
+		// character and let mode be "relative-minus"."
+		} else if (value[0] == "-") {
+			value = value.slice(1);
+			mode = "relative-minus";
+		// "Otherwise, let mode be "absolute"."
+		} else {
+			mode = "absolute";
+		}
+
+		// "Apply the rules for parsing non-negative integers to value, and
+		// let number be the result."
+		//
+		// Another cheap hack.
+		var num = parseInt(value);
+
+		// "If mode is "relative-plus", add three to number."
+		if (mode == "relative-plus") {
+			num += 3;
+		}
+
+		// "If mode is "relative-minus", negate number, then add three to
+		// it."
+		if (mode == "relative-minus") {
+			num = 3 - num;
+		}
+
+		// "If number is less than one, let number equal 1."
+		if (num < 1) {
+			num = 1;
+		}
+
+		// "If number is greater than seven, let number equal 7."
+		if (num > 7) {
+			num = 7;
+		}
+
+		// "Set value to the string here corresponding to number:" [table
+		// omitted]
+		value = {
+			1: "xx-small",
+			2: "small",
+			3: "medium",
+			4: "large",
+			5: "x-large",
+			6: "xx-large",
+			7: "xxx-large"
+		}[num];
+	}
+
+	return value;
+}
+
 commands.fontsize = {
 	action: function(value) {
 		// "If value is the empty string, raise a SYNTAX_ERR exception."
@@ -3098,71 +3183,7 @@
 			throw "SYNTAX_ERR";
 		}
 
-		// "Strip leading and trailing whitespace from value."
-		//
-		// Cheap hack, not following the actual algorithm.
-		value = value.trim();
-
-		// "If value is a valid floating point number, or would be a valid
-		// floating point number if a single leading "+" character were
-		// stripped:"
-		if (/^[-+]?[0-9]+(\.[0-9]+)?([eE][-+]?[0-9]+)?$/.test(value)) {
-			var mode;
-
-			// "If the first character of value is "+", delete the character
-			// and let mode be "relative-plus"."
-			if (value[0] == "+") {
-				value = value.slice(1);
-				mode = "relative-plus";
-			// "Otherwise, if the first character of value is "-", delete the
-			// character and let mode be "relative-minus"."
-			} else if (value[0] == "-") {
-				value = value.slice(1);
-				mode = "relative-minus";
-			// "Otherwise, let mode be "absolute"."
-			} else {
-				mode = "absolute";
-			}
-
-			// "Apply the rules for parsing non-negative integers to value, and
-			// let number be the result."
-			//
-			// Another cheap hack.
-			var num = parseInt(value);
-
-			// "If mode is "relative-plus", add three to number."
-			if (mode == "relative-plus") {
-				num += 3;
-			}
-
-			// "If mode is "relative-minus", negate number, then add three to
-			// it."
-			if (mode == "relative-minus") {
-				num = 3 - num;
-			}
-
-			// "If number is less than one, let number equal 1."
-			if (num < 1) {
-				num = 1;
-			}
-
-			// "If number is greater than seven, let number equal 7."
-			if (num > 7) {
-				num = 7;
-			}
-
-			// "Set value to the string here corresponding to number:" [table
-			// omitted]
-			value = {
-				1: "xx-small",
-				2: "small",
-				3: "medium",
-				4: "large",
-				5: "x-large",
-				6: "xx-large",
-				7: "xxx-large"
-			}[num];
-		}
+		value = normalizeFontSize(value);
 
 		// "If value is not one of the strings "xx-small", "x-small", "small",
 		// "medium", "large", "x-large", "xx-large", "xxx-large", and is not a
@@ -3201,43 +3222,48 @@
 		}
 		var pixelSize = parseInt(getEffectiveCommandValue(node, "fontsize"));
 
-		// "Let returned size be 1."
-		var returnedSize = 1;
-
-		// "While returned size is less than 7:"
-		while (returnedSize < 7) {
-			// "Let lower bound be the resolved value of "font-size" in pixels
-			// of a font element whose size attribute is set to returned size."
-			var font = document.createElement("font");
-			font.size = returnedSize;
-			document.body.appendChild(font);
-			var lowerBound = parseInt(getComputedStyle(font).fontSize);
-
-			// "Let upper bound be the resolved value of "font-size" in pixels
-			// of a font element whose size attribute is set to one plus
-			// returned size."
-			font.size = 1 + returnedSize;
-			var upperBound = parseInt(getComputedStyle(font).fontSize);
-			document.body.removeChild(font);
-
-			// "Let average be the average of upper bound and lower bound."
-			var average = (upperBound + lowerBound)/2;
-
-			// "If pixel size is less than average, return the one-element
-			// string consisting of the digit returned size."
-			if (pixelSize < average) {
-				return String(returnedSize);
-			}
-
-			// "Add one to returned size."
-			returnedSize++;
-		}
-
-		// "Return "7"."
-		return "7";
+		// "Return the legacy font size for pixel size."
+		return getLegacyFontSize(pixelSize);
 	}, relevantCssProperty: "fontSize"
 };
 
+function getLegacyFontSize(pixelSize) {
+	// "Let returned size be 1."
+	var returnedSize = 1;
+
+	// "While returned size is less than 7:"
+	while (returnedSize < 7) {
+		// "Let lower bound be the resolved value of "font-size" in pixels
+		// of a font element whose size attribute is set to returned size."
+		var font = document.createElement("font");
+		font.size = returnedSize;
+		document.body.appendChild(font);
+		var lowerBound = parseInt(getComputedStyle(font).fontSize);
+
+		// "Let upper bound be the resolved value of "font-size" in pixels
+		// of a font element whose size attribute is set to one plus
+		// returned size."
+		font.size = 1 + returnedSize;
+		var upperBound = parseInt(getComputedStyle(font).fontSize);
+		document.body.removeChild(font);
+
+		// "Let average be the average of upper bound and lower bound."
+		var average = (upperBound + lowerBound)/2;
+
+		// "If pixel size is less than average, return the one-element
+		// string consisting of the digit returned size."
+		if (pixelSize < average) {
+			return String(returnedSize);
+		}
+
+		// "Add one to returned size."
+		returnedSize++;
+	}
+
+	// "Return "7"."
+	return "7";
+}
+
 //@}
 ///// The foreColor command /////
 //@{
--- a/source.html	Thu Jul 14 13:18:24 2011 -0600
+++ b/source.html	Thu Jul 14 14:21:49 2011 -0600
@@ -1976,8 +1976,8 @@
     opaque with red, green, and blue components in the range 0 to 255:
     <!-- See comment for foreColor for discussion. -->
 
-    <p class=XXX>Check this more carefully for what happens if the components
-    are not integers or are out-of-range.
+    <!-- TODO: Define more carefully what happens when things are out of range
+    or not integers or whatever. -->
 
     <ol>
       <li>Let <var>new parent</var> be the result of calling <code
@@ -2248,12 +2248,15 @@
       <dt>underline<dd>True if <var>new value</var> is "underline", false otherwise.
     </dl>
 
-    <li>If <var>command</var> has a <span>value</span> specified, unset the
-    <span>value override</span> if <var>new value</var> is null, and set the
-    <span>value override</span> to <var>new value</var> if it is not null.
-
-    <p class=XXX>This doesn't work as-is for fontSize, because that uses a
-    different format for the value.
+    <li>If <var>new value</var> is null, unset the <span>value override</span>
+    (if any).
+
+    <li>Otherwise, if <var>command</var> is "fontSize", set the <span>value
+    override</span> to the <span>legacy font size for</span> the result of
+    converting <var>new value</var> to pixels.
+
+    <li>Otherwise, if <var>command</var> has a <span>value</span> specified,
+    set the <span>value override</span> to <var>new value</var>.
 
     <li>Abort these steps.
   </ol>
@@ -2721,6 +2724,15 @@
   cannot be null, since the boundary point node of a selection must always be
   either an element or a text node that's the child of an element.
 
+  <li>Return the <span>legacy font size for</span> <var>pixel size</var>.
+</ol>
+
+<p><span>Relevant CSS property</span>: "font-size"
+
+<p>The <dfn>legacy font size for</dfn> an integer <var>pixel size</var> is
+returned by the following algorithm:
+
+<ol>
   <li>Let <var>returned size</var> be 1.
 
   <li>While <var>returned size</var> is less than 7:
@@ -2745,9 +2757,6 @@
 
   <li>Return "7".
 </ol>
-
-<p><span>Relevant CSS property</span>: "font-size"
-
 <!-- @} -->
 <h3><dfn>The <code title>foreColor</code> command</dfn></h3>
 <!-- @{ -->
--- a/tests.js	Thu Jul 14 13:18:24 2011 -0600
+++ b/tests.js	Thu Jul 14 14:21:49 2011 -0600
@@ -3440,12 +3440,23 @@
 	beforeDiv.appendChild(document.createTextNode(", "));
 	afterDiv.appendChild(document.createTextNode(", "));
 
-	// Hack to get highlighting working right for colors
 	if (command == "backcolor" || command == "forecolor" || command == "hilitecolor") {
 		if (/^([0-9a-fA-F]{3}){1,2}$/.test(value)) {
 			value = "#" + value;
 		}
 	}
+	if (command == "fontsize") {
+		value = normalizeFontSize(value);
+		var font = document.createElement("font");
+		document.body.appendChild(font);
+		if (value == "xxx-large") {
+			font.size = 7;
+		} else {
+			font.style.fontSize = value;
+		}
+		value = getLegacyFontSize(parseInt(getComputedStyle(font).fontSize));
+		document.body.removeChild(font);
+	}
 
 	beforeDiv.appendChild(document.createElement("span"));
 	afterDiv.appendChild(document.createElement("span"));