Bug 745685 - Parse <font size> per HTML5; r=bz

content/html/content/src/nsHTMLFontElement.cpp

@@ -48,6 +48,7 @@
 #include "nsRuleData.h"
 #include "nsIDocument.h"
 #include "nsAlgorithm.h"
+#include "nsContentUtils.h"
 
 using namespace mozilla;
 
@@ -116,32 +117,59 @@ NS_IMPL_STRING_ATTR(nsHTMLFontElement, Color, color)
 NS_IMPL_STRING_ATTR(nsHTMLFontElement, Face, face)
 NS_IMPL_STRING_ATTR(nsHTMLFontElement, Size, size)
 
-static const nsAttrValue::EnumTable kRelFontSizeTable[] = {
-  { "-10", -10 },
-  { "-9", -9 },
-  { "-8", -8 },
-  { "-7", -7 },
-  { "-6", -6 },
-  { "-5", -5 },
-  { "-4", -4 },
-  { "-3", -3 },
-  { "-2", -2 },
-  { "-1", -1 },
-  { "-0", 0 },
-  { "+0", 0 },
-  { "+1", 1 },
-  { "+2", 2 },
-  { "+3", 3 },
-  { "+4", 4 },
-  { "+5", 5 },
-  { "+6", 6 },
-  { "+7", 7 },
-  { "+8", 8 },
-  { "+9", 9 },
-  { "+10", 10 },
-  { 0 }
-};
 
+// Returns 1 to 7, or 0 on error.  Follows HTML spec as of April 16, 2012.
+static PRInt32
+ParseLegacyFontSize(const nsAString& aValue)
+{
+  nsAString::const_iterator iter, end;
+  aValue.BeginReading(iter);
+  aValue.EndReading(end);
+
+  while (iter != end && nsContentUtils::IsHTMLWhitespace(*iter)) {
+    ++iter;
+  }
+
+  if (iter == end) {
+    return 0;
+  }
+
+  bool relative = false;
+  bool negate = false;
+  if (*iter == PRUnichar('-')) {
+    relative = true;
+    negate = true;
+    ++iter;
+  } else if (*iter == PRUnichar('+')) {
+    relative = true;
+    ++iter;
+  }
+
+  if (*iter < PRUnichar('0') || *iter > PRUnichar('9')) {
+    return 0;
+  }
+
+  // We don't have to worry about overflow, since we can bail out as soon as
+  // we're bigger than 7.
+  PRInt32 value = 0;
+  while (iter != end && *iter >= PRUnichar('0') && *iter <= PRUnichar('9')) {
+    value = 10*value + (*iter - PRUnichar('0'));
+    if (value >= 7) {
+      break;
+    }
+    ++iter;
+  }
+
+  if (relative) {
+    if (negate) {
+      value = 3 - value;
+    } else {
+      value = 3 + value;
+    }
+  }
+
+  return clamped(value, 1, 7);
+}
 
 bool
 nsHTMLFontElement::ParseAttribute(PRInt32 aNamespaceID,
@@ -151,26 +179,12 @@ nsHTMLFontElement::ParseAttribute(PRInt32 aNamespaceID,
 {
   if (aNamespaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::size) {
-      nsAutoString tmp(aValue);
-      tmp.CompressWhitespace(true, true);
-      PRUnichar ch = tmp.IsEmpty() ? 0 : tmp.First();
-      if ((ch == '+' || ch == '-')) {
-          if (aResult.ParseEnumValue(aValue, kRelFontSizeTable, false))
-              return true;
-
-          // truncate after digit, then parse it again.
-          PRUint32 i;
-          for (i = 1; i < tmp.Length(); i++) {
-              ch = tmp.CharAt(i);
-              if (!nsCRT::IsAsciiDigit(ch)) {
-                  tmp.Truncate(i);
-                  break;
-              }
-          }
-          return aResult.ParseEnumValue(tmp, kRelFontSizeTable, false);
+      PRInt32 size = ParseLegacyFontSize(aValue);
+      if (size) {
+        aResult.SetTo(size, &aValue);
+        return true;
       }
-
-      return aResult.ParseIntValue(aValue);
+      return false;
     }
     if (aAttribute == nsGkAtoms::pointSize ||
         aAttribute == nsGkAtoms::fontWeight) {
@@ -207,20 +221,10 @@ MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
       if (value && value->Type() == nsAttrValue::eInteger)
         fontSize->SetFloatValue((float)value->GetIntegerValue(), eCSSUnit_Point);
       else {
-        // size: int, enum , 
+        // size: int
         value = aAttributes->GetAttr(nsGkAtoms::size);
-        if (value) {
-          nsAttrValue::ValueType unit = value->Type();
-          if (unit == nsAttrValue::eInteger || unit == nsAttrValue::eEnum) { 
-            PRInt32 size;
-            if (unit == nsAttrValue::eEnum) // int (+/-)
-              size = value->GetEnumValue() + 3;
-            else
-              size = value->GetIntegerValue();
-
-            size = clamped(size, 1, 7);
-            fontSize->SetIntValue(size, eCSSUnit_Enumerated);
-          }
+        if (value && value->Type() == nsAttrValue::eInteger) {
+          fontSize->SetIntValue(value->GetIntegerValue(), eCSSUnit_Enumerated);
         }
       }
     }

content/html/content/test/Makefile.in

@@ -301,6 +301,7 @@ _TEST_FILES = \
 		test_object_plugin_nav.html \
 		test_bug742030.html \
 		test_bug742549.html \
+		test_bug745685.html \
 		$(NULL)
 
 _BROWSER_TEST_FILES = \

content/html/content/test/test_bug745685.html

@@ -0,0 +1,105 @@
+<!doctype html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=745685
+-->
+<title>Test for Bug 745685</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=745685">Mozilla Bug 745685</a>
+<font>Test text</font>
+<font size=1>1</font>
+<font size=2>2</font>
+<font size=3>3</font>
+<font size=4>4</font>
+<font size=5>5</font>
+<font size=6>6</font>
+<font size=7>7</font>
+<script>
+/** Test for Bug 745685 **/
+
+var referenceSizes = {};
+for (var i = 1; i <= 7; i++) {
+  referenceSizes[i] =
+    getComputedStyle(document.querySelector('[size="' + i + '"]'))
+    .fontSize;
+  if (i > 1) {
+    isnot(referenceSizes[i], referenceSizes[i - 1],
+          "Sanity check: different <font size>s give different .fontSize");
+  }
+}
+
+function testFontSize(input, expected) {
+  var font = document.querySelector("font");
+  font.setAttribute("size", input);
+  is(font.getAttribute("size"), input,
+     "Setting doesn't round-trip (.getAttribute)");
+  is(font.size, input,
+     "Setting doesn't round-trip (.size)");
+  is(getComputedStyle(font).fontSize, referenceSizes[expected],
+     'Incorrect size for "' + input + '" : expected the same as ' + expected);
+}
+
+function testFontSizes(input, expected) {
+  testFontSize(input, expected);
+  // Leading whitespace
+  testFontSize(" " + input, expected);
+  testFontSize("\t" + input, expected);
+  testFontSize("\n" + input, expected);
+  testFontSize("\f" + input, expected);
+  testFontSize("\r" + input, expected);
+  // Trailing garbage
+  testFontSize(input + "abcd", expected);
+  testFontSize(input + ".5", expected);
+  testFontSize(input + "e2", expected);
+}
+
+// Parse error
+testFontSizes("", 3);
+
+// No sign
+testFontSizes("0", 1);
+testFontSizes("1", 1);
+testFontSizes("2", 2);
+testFontSizes("3", 3);
+testFontSizes("4", 4);
+testFontSizes("5", 5);
+testFontSizes("6", 6);
+testFontSizes("7", 7);
+testFontSizes("8", 7);
+testFontSizes("9", 7);
+testFontSizes("10", 7);
+testFontSizes("10000000000000000000000", 7);
+
+// Minus sign
+testFontSizes("-0", 3);
+testFontSizes("-1", 2);
+testFontSizes("-2", 1);
+testFontSizes("-3", 1);
+testFontSizes("-4", 1);
+testFontSizes("-5", 1);
+testFontSizes("-6", 1);
+testFontSizes("-7", 1);
+testFontSizes("-8", 1);
+testFontSizes("-9", 1);
+testFontSizes("-10", 1);
+testFontSizes("-10000000000000000000000", 1);
+
+// Plus sign
+testFontSizes("+0", 3);
+testFontSizes("+1", 4);
+testFontSizes("+2", 5);
+testFontSizes("+3", 6);
+testFontSizes("+4", 7);
+testFontSizes("+5", 7);
+testFontSizes("+6", 7);
+testFontSizes("+7", 7);
+testFontSizes("+8", 7);
+testFontSizes("+9", 7);
+testFontSizes("+10", 7);
+testFontSizes("+10000000000000000000000", 7);
+
+// Non-HTML5 whitespace
+testFontSize("\b1", 3);
+testFontSize("\v1", 3);
+testFontSize("\0u00a01", 3);
+</script>