Only accept 1-7 as input values for fontSize
authorAryeh Gregor <AryehGregor+gitcommit@gmail.com>
Thu, 22 Sep 2011 14:39:11 -0600
changeset 619 031902d853b9
parent 618 e60077322f8a
child 620 ba1b5b4672db
Only accept 1-7 as input values for fontSize

Fixes: http://www.w3.org/Bugs/Public/show_bug.cgi?id=14251
conformancetest/data.js
editing.html
implementation.js
source.html
tests.js
--- a/conformancetest/data.js	Thu Sep 22 14:03:58 2011 -0600
+++ b/conformancetest/data.js	Thu Sep 22 14:39:11 2011 -0600
@@ -4729,20 +4729,20 @@
 	{"stylewithcss":[false,false,"",false,true,""],"fontsize":[false,false,"3",false,false,"3"]}],
 ["foo[bar]baz",
 	[["stylewithcss","false"],["fontsize","20pt"]],
-	"foo<span style=\"font-size:20pt\">[bar]</span>baz",
-	{"stylewithcss":[false,true,"",false,false,""],"fontsize":[false,false,"3",false,false,"5"]}],
+	"foo[bar]baz",
+	{"stylewithcss":[false,true,"",false,false,""],"fontsize":[false,false,"3",false,false,"3"]}],
 ["foo[bar]baz",
 	[["stylewithcss","true"],["fontsize","20pt"]],
-	"foo<span style=\"font-size:20pt\">[bar]</span>baz",
-	{"stylewithcss":[false,false,"",false,true,""],"fontsize":[false,false,"3",false,false,"5"]}],
+	"foo[bar]baz",
+	{"stylewithcss":[false,false,"",false,true,""],"fontsize":[false,false,"3",false,false,"3"]}],
 ["foo[bar]baz",
 	[["stylewithcss","false"],["fontsize","xx-large"]],
-	"foo<font size=\"6\">[bar]</font>baz",
-	{"stylewithcss":[false,true,"",false,false,""],"fontsize":[false,false,"3",false,false,"6"]}],
+	"foo[bar]baz",
+	{"stylewithcss":[false,true,"",false,false,""],"fontsize":[false,false,"3",false,false,"3"]}],
 ["foo[bar]baz",
 	[["stylewithcss","true"],["fontsize","xx-large"]],
-	"foo<span style=\"font-size:xx-large\">[bar]</span>baz",
-	{"stylewithcss":[false,false,"",false,true,""],"fontsize":[false,false,"3",false,false,"6"]}],
+	"foo[bar]baz",
+	{"stylewithcss":[false,false,"",false,true,""],"fontsize":[false,false,"3",false,false,"3"]}],
 ["foo[bar]baz",
 	[["stylewithcss","false"],["fontsize"," 1 "]],
 	"foo<font size=\"1\">[bar]</font>baz",
--- a/editing.html	Thu Sep 22 14:03:58 2011 -0600
+++ b/editing.html	Thu Sep 22 14:39:11 2011 -0600
@@ -3316,82 +3316,64 @@
 </dl>
 
 <p>What all of these have in common is that they force the author to deal with
-legacy font values and don't let them use CSS.  This is undesirable, so I
-ignore how implementations behave.  Practically any value that did the same
-thing in IE and Firefox should still do the same thing here, so I'm only
-respecifying non-interoperable behavior anyway.
+legacy font values and don't let them use CSS.  This is undesirable, but to
+avoid it we'd really have to create a new command.  If nothing else, the value
+returned by <code title="">queryCommandValue()</code> has to be numeric, so authors can't
+really use the command sanely no matter what we do.  See <a href="http://www.w3.org/Bugs/Public/show_bug.cgi?id=14251">bug 14251</a>.
 </div>
 
 <p><a href=#action>Action</a>:
 
 <ol>
-  <li>If <var title="">value</var> is the empty string, abort these steps and do
-  nothing.
-
   <li><a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#strip-leading-and-trailing-whitespace>Strip leading and trailing whitespace</a>
   from <var title="">value</var>.
 
-  <li>If <var title="">value</var> is a <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#valid-floating-point-number>valid floating point
-  number</a>, or would be a <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#valid-floating-point-number>valid floating point
-  number</a> if a single leading "+" character were stripped:
-
-  <ol>
-    <li>If the first character of <var title="">value</var> is "+", delete the character
-    and let <var title="">mode</var> be "relative-plus".
-
-    <li>Otherwise, if the first character of <var title="">value</var> is "-", delete
-    the character and let <var title="">mode</var> be "relative-minus".
-
-    <li>Otherwise, let <var title="">mode</var> be "absolute".
-
-    <li>Apply the <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#rules-for-parsing-non-negative-integers>rules for parsing non-negative
-    integers</a> to <var title="">value</var>, and let <var title="">number</var> be the
-    result.
-
-    <li>If <var title="">mode</var> is "relative-plus", add three to <var title="">number</var>.
-
-    <li>If <var title="">mode</var> is "relative-minus", negate <var title="">number</var>, then
-    add three to it.
-
-    <li>If <var title="">number</var> is less than one, let <var title="">number</var> equal 1.
-
-    <li>If <var title="">number</var> is greater than seven, let <var title="">number</var> equal
-    7.
-
-    <li>Set <var title="">value</var> to the string here corresponding to
-    <var title="">number</var>:
-
-    <p class=comments> The entry for 7 here is an issue: there's no CSS value
-    that corresponds to it.  Even if we got one added to the drafts, it
-    wouldn't be backward-compatible to use it.  WebKit is the only engine that
-    supports CSS output for fontSize, and it uses -webkit-xxx-large in this
-    case, which is unworkable.  Instead, we just always output a font tag for
-    size 7.  If authors want conforming markup, they'll need to give CSS sizes
-    above size 7, not legacy sizes.
-
-    <ul>
-      <li>1: xx-small
-      <li>2: small
-      <li>3: medium
-      <li>4: large
-      <li>5: x-large
-      <li>6: xx-large
-      <li>7: xxx-large
-    </ul>
-  </ol>
-
-  <li>
-  <p class=comments> Not sure this is the best way to do it.  We don't want to
-  allow relative lengths, because those can have very weird user-visible
-  behavior.  For instance, a size of 2em would sometimes double the text size,
-  but if you applied it a second time it would do nothing, but if you
-  deselected one character it would suddenly double the size again.  Current
-  UAs just only allow numeric values.  There's no harm in allowing "x-small"
-  and absolute sizes, I don't think.
-
-  <p>If <var title="">value</var> is not one of the strings "xx-small", "x-small",
-  "small", "medium", "large", "x-large", "xx-large", "xxx-large", and is not a
-  valid CSS absolute length, then abort these steps and do nothing.
+  <li>If <var title="">value</var> is not a <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#valid-floating-point-number>valid floating
+  point number</a>, and would not be a <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#valid-floating-point-number>valid
+  floating point number</a> if a single leading "+" character were stripped,
+  abort these steps and do nothing.
+
+  <li>If the first character of <var title="">value</var> is "+", delete the character
+  and let <var title="">mode</var> be "relative-plus".
+
+  <li>Otherwise, if the first character of <var title="">value</var> is "-", delete the
+  character and let <var title="">mode</var> be "relative-minus".
+
+  <li>Otherwise, let <var title="">mode</var> be "absolute".
+
+  <li>Apply the <a class=external data-anolis-spec=html href=http://www.whatwg.org/html/#rules-for-parsing-non-negative-integers>rules for parsing non-negative
+  integers</a> to <var title="">value</var>, and let <var title="">number</var> be the result.
+
+  <li>If <var title="">mode</var> is "relative-plus", add three to <var title="">number</var>.
+
+  <li>If <var title="">mode</var> is "relative-minus", negate <var title="">number</var>, then
+  add three to it.
+
+  <li>If <var title="">number</var> is less than one, let <var title="">number</var> equal 1.
+
+  <li>If <var title="">number</var> is greater than seven, let <var title="">number</var> equal
+  7.
+
+  <li>Set <var title="">value</var> to the string here corresponding to
+  <var title="">number</var>:
+
+  <p class=comments>The entry for 7 here is an issue: there's no CSS value that
+  corresponds to it.  Even if we got one added to the drafts, it wouldn't be
+  backward-compatible to use it.  WebKit is the only engine that supports CSS
+  output for fontSize, and it uses -webkit-xxx-large in this case, which is
+  unworkable.  Instead, we just always output a font tag for size 7.  If
+  authors want conforming markup, they'll need to give CSS sizes above size 7,
+  not legacy sizes.
+
+  <ul>
+    <li>1: xx-small
+    <li>2: small
+    <li>3: medium
+    <li>4: large
+    <li>5: x-large
+    <li>6: xx-large
+    <li>7: xxx-large
+  </ul>
 
   <li><a href="#set-the-selection's-value">Set the selection's value</a> to <var title="">value</var>.
 </ol>
--- a/implementation.js	Thu Sep 22 14:03:58 2011 -0600
+++ b/implementation.js	Thu Sep 22 14:39:11 2011 -0600
@@ -3139,93 +3139,82 @@
 //@{
 
 // Helper function for fontSize's action plus queryOutputHelper.  It's just the
-// middle of fontSize's action, ripped out into its own function.
+// middle of fontSize's action, ripped out into its own function.  Returns null
+// if the size is invalid.
 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];
-	}
+	// "If value is not a valid floating point number, and would not be a valid
+	// floating point number if a single leading "+" character were stripped,
+	// abort these steps and do nothing."
+	if (!/^[-+]?[0-9]+(\.[0-9]+)?([eE][-+]?[0-9]+)?$/.test(value)) {
+		return null;
+	}
+
+	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, abort these steps and do nothing."
-		if (value === "") {
-			return;
-		}
-
 		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
-		// valid CSS absolute length, then abort these steps and do nothing."
-		//
-		// More cheap hacks to skip valid CSS absolute length checks.
-		if (["xx-small", "x-small", "small", "medium", "large", "x-large", "xx-large", "xxx-large"].indexOf(value) == -1
-		&& !/^[0-9]+(\.[0-9]+)?(cm|mm|in|pt|pc)$/.test(value)) {
+		if (value === null) {
 			return;
 		}
 
@@ -3262,7 +3251,9 @@
 	// For convenience in other places in my code, I handle all sizes, not just
 	// pixel sizes as the spec says.  This means pixel sizes have to be passed
 	// in suffixed with "px", not as plain numbers.
-	size = normalizeFontSize(size);
+	if (normalizeFontSize(size) !== null) {
+		return cssSizeToLegacy(normalizeFontSize(size));
+	}
 
 	if (["xx-small", "x-small", "small", "medium", "large", "x-large", "xx-large", "xxx-large"].indexOf(size) == -1
 	&& !/^[0-9]+(\.[0-9]+)?(cm|mm|in|pt|pc|px)$/.test(size)) {
--- a/source.html	Thu Sep 22 14:03:58 2011 -0600
+++ b/source.html	Thu Sep 22 14:39:11 2011 -0600
@@ -3328,82 +3328,65 @@
 </dl>
 
 <p>What all of these have in common is that they force the author to deal with
-legacy font values and don't let them use CSS.  This is undesirable, so I
-ignore how implementations behave.  Practically any value that did the same
-thing in IE and Firefox should still do the same thing here, so I'm only
-respecifying non-interoperable behavior anyway.
+legacy font values and don't let them use CSS.  This is undesirable, but to
+avoid it we'd really have to create a new command.  If nothing else, the value
+returned by {{code|queryCommandValue()}} has to be numeric, so authors can't
+really use the command sanely no matter what we do.  See <a
+href=http://www.w3.org/Bugs/Public/show_bug.cgi?id=14251>bug 14251</a>.
 </div>
 
 <p><span>Action</span>:
 
 <ol>
-  <li>If <var>value</var> is the empty string, abort these steps and do
-  nothing.
-
   <li><span data-anolis-spec=html>Strip leading and trailing whitespace</span>
   from <var>value</var>.
 
-  <li>If <var>value</var> is a <span data-anolis-spec=html>valid floating point
-  number</span>, or would be a <span data-anolis-spec=html>valid floating point
-  number</span> if a single leading "+" character were stripped:
-
-  <ol>
-    <li>If the first character of <var>value</var> is "+", delete the character
-    and let <var>mode</var> be "relative-plus".
-
-    <li>Otherwise, if the first character of <var>value</var> is "-", delete
-    the character and let <var>mode</var> be "relative-minus".
-
-    <li>Otherwise, let <var>mode</var> be "absolute".
-
-    <li>Apply the <span data-anolis-spec=html>rules for parsing non-negative
-    integers</span> to <var>value</var>, and let <var>number</var> be the
-    result.
-
-    <li>If <var>mode</var> is "relative-plus", add three to <var>number</var>.
-
-    <li>If <var>mode</var> is "relative-minus", negate <var>number</var>, then
-    add three to it.
-
-    <li>If <var>number</var> is less than one, let <var>number</var> equal 1.
-
-    <li>If <var>number</var> is greater than seven, let <var>number</var> equal
-    7.
-
-    <li>Set <var>value</var> to the string here corresponding to
-    <var>number</var>:
-
-    <p class=comments> The entry for 7 here is an issue: there's no CSS value
-    that corresponds to it.  Even if we got one added to the drafts, it
-    wouldn't be backward-compatible to use it.  WebKit is the only engine that
-    supports CSS output for fontSize, and it uses -webkit-xxx-large in this
-    case, which is unworkable.  Instead, we just always output a font tag for
-    size 7.  If authors want conforming markup, they'll need to give CSS sizes
-    above size 7, not legacy sizes.
-
-    <ul>
-      <li>1: xx-small
-      <li>2: small
-      <li>3: medium
-      <li>4: large
-      <li>5: x-large
-      <li>6: xx-large
-      <li>7: xxx-large
-    </ul>
-  </ol>
-
-  <li>
-  <p class=comments> Not sure this is the best way to do it.  We don't want to
-  allow relative lengths, because those can have very weird user-visible
-  behavior.  For instance, a size of 2em would sometimes double the text size,
-  but if you applied it a second time it would do nothing, but if you
-  deselected one character it would suddenly double the size again.  Current
-  UAs just only allow numeric values.  There's no harm in allowing "x-small"
-  and absolute sizes, I don't think.
-
-  <p>If <var>value</var> is not one of the strings "xx-small", "x-small",
-  "small", "medium", "large", "x-large", "xx-large", "xxx-large", and is not a
-  valid CSS absolute length, then abort these steps and do nothing.
+  <li>If <var>value</var> is not a <span data-anolis-spec=html>valid floating
+  point number</span>, and would not be a <span data-anolis-spec=html>valid
+  floating point number</span> if a single leading "+" character were stripped,
+  abort these steps and do nothing.
+
+  <li>If the first character of <var>value</var> is "+", delete the character
+  and let <var>mode</var> be "relative-plus".
+
+  <li>Otherwise, if the first character of <var>value</var> is "-", delete the
+  character and let <var>mode</var> be "relative-minus".
+
+  <li>Otherwise, let <var>mode</var> be "absolute".
+
+  <li>Apply the <span data-anolis-spec=html>rules for parsing non-negative
+  integers</span> to <var>value</var>, and let <var>number</var> be the result.
+
+  <li>If <var>mode</var> is "relative-plus", add three to <var>number</var>.
+
+  <li>If <var>mode</var> is "relative-minus", negate <var>number</var>, then
+  add three to it.
+
+  <li>If <var>number</var> is less than one, let <var>number</var> equal 1.
+
+  <li>If <var>number</var> is greater than seven, let <var>number</var> equal
+  7.
+
+  <li>Set <var>value</var> to the string here corresponding to
+  <var>number</var>:
+
+  <p class=comments>The entry for 7 here is an issue: there's no CSS value that
+  corresponds to it.  Even if we got one added to the drafts, it wouldn't be
+  backward-compatible to use it.  WebKit is the only engine that supports CSS
+  output for fontSize, and it uses -webkit-xxx-large in this case, which is
+  unworkable.  Instead, we just always output a font tag for size 7.  If
+  authors want conforming markup, they'll need to give CSS sizes above size 7,
+  not legacy sizes.
+
+  <ul>
+    <li>1: xx-small
+    <li>2: small
+    <li>3: medium
+    <li>4: large
+    <li>5: x-large
+    <li>6: xx-large
+    <li>7: xxx-large
+  </ul>
 
   <li><span>Set the selection's value</span> to <var>value</var>.
 </ol>
--- a/tests.js	Thu Sep 22 14:03:58 2011 -0600
+++ b/tests.js	Thu Sep 22 14:39:11 2011 -0600
@@ -4390,15 +4390,16 @@
 			value = "#" + value;
 		}
 	} else if (command == "fontsize") {
-		beforeValue = legacySizeToCss(beforeValue);
-		afterValue = legacySizeToCss(afterValue);
-		value = legacySizeToCss(getLegacyFontSize(value));
+		value = normalizeFontSize(value);
+		if (value !== null) {
+			value = String(cssSizeToLegacy(value));
+		}
 	} else if (command == "formatblock") {
 		value = value.replace(/^<(.*)>$/, "$1").toLowerCase();
 	}
 
 	if (((command == "backcolor" || command == "forecolor" || command == "hilitecolor") && value.toLowerCase() == "currentcolor")
-	|| (command == "fontsize" && value === undefined)
+	|| (command == "fontsize" && value === null)
 	|| (command == "formatblock" && formattableBlockNames.indexOf(value.replace(/^<(.*)>$/, "$1").trim()) == -1)) {
 		afterDiv.lastChild.className = beforeValue === afterValue
 			? "good-result"