Bug 280443 p4 - properly escape unquoted font family names. r=heycam

layout/style/nsStyleUtil.cpp

@@ -143,6 +143,34 @@ nsStyleUtil::AppendEscapedCSSIdent(const nsAString& aIdent, nsAString& aReturn)
   return true;
 }
 
+// unquoted family names must be a sequence of idents
+// so escape any parts that require escaping
+static void
+AppendUnquotedFamilyName(const nsAString& aFamilyName, nsAString& aResult)
+{
+  const char16_t *p, *p_end;
+  aFamilyName.BeginReading(p);
+  aFamilyName.EndReading(p_end);
+
+   bool moreThanOne = false;
+   while (p < p_end) {
+     const char16_t* identStart = p;
+     while (++p != p_end && *p != ' ')
+       /* nothing */ ;
+
+     nsDependentSubstring ident(identStart, p);
+     if (!ident.IsEmpty()) {
+       if (moreThanOne) {
+         aResult.Append(' ');
+       }
+       nsStyleUtil::AppendEscapedCSSIdent(ident, aResult);
+       moreThanOne = true;
+     }
+
+     ++p;
+  }
+}
+
 /* static */ void
 nsStyleUtil::AppendEscapedCSSFontFamilyList(
   const mozilla::FontFamilyList& aFamilyList,
@@ -157,9 +185,7 @@ nsStyleUtil::AppendEscapedCSSFontFamilyList(
     const FontFamilyName& name = fontlist[i];
     switch (name.mType) {
       case eFamily_named:
-        // xxx - need to do appropriate escaping, done in later patch
-        // iterate over idents, escape each ident
-        aResult.Append(name.mName);
+        AppendUnquotedFamilyName(name.mName, aResult);
         break;
       case eFamily_named_quoted:
         AppendEscapedCSSString(name.mName, aResult);

layout/style/test/test_font_family_parsing.html

@@ -55,14 +55,13 @@ var testFontFamilyLists = [
   { namelist: "arial, helvetica, 'times' new roman, sans-serif", invalid: true },
   { namelist: "arial, helvetica, \"times\" new roman, sans-serif", invalid: true },
 
-  // bug 660397 - quotes contained within family names are not escaped
-  // { namelist: "arial, helvetica, \"\\\"times new roman\", sans-serif" },
+  { namelist: "arial, helvetica, \"\\\"times new roman\", sans-serif" },
   { namelist: "arial, helvetica, '\\\"times new roman', sans-serif" },
   { namelist: "arial, helvetica, times 'new' roman, sans-serif", invalid: true },
   { namelist: "arial, helvetica, times \"new\" roman, sans-serif", invalid: true },
-  // { namelist: "\"simple", invalid: true, single: true },
-  // { namelist: "\\\"simple", single: true },
-  // { namelist: "\"\\\"simple\"", single: true },
+  { namelist: "\"simple", single: true },
+  { namelist: "\\\"simple", single: true },
+  { namelist: "\"\\\"simple\"", single: true },
   { namelist: "İsimple", single: true },
   { namelist: "ßsimple", single: true },
   { namelist: "ẙsimple", single: true },
@@ -71,26 +70,18 @@ var testFontFamilyLists = [
   { namelist: "\\s imple", single: true },
   { namelist: "\\073 imple", single: true },
 
-  // bug 475216 - css serialization doesn't escape characters that need escaping
-  // { namelist: "\\035 simple", single: true },
+  { namelist: "\\035 simple", single: true },
   { namelist: "sim\\035 ple", single: true },
-  // { namelist: "simple\\02cinitial", single: true },
-  // { namelist: "simple, \\02cinitial" },
-  // { namelist: "sim\\020 \\035 ple", single: true },
-  // { namelist: "sim\\020 5ple", single: true },
-  // { namelist: "\\;", single: true },
-  // { namelist: "\\;,\\;", single: true },
-  // { namelist: "\\,\\;", single: true },
-  // { namelist: "\\{", single: true },
-  // { namelist: "\\{\\;", single: true },
-  // { namelist: "\\}", single: true },
-  // { namelist: "\\}\\;", single: true },
-  // { namelist: "\\@simple", single: true },
-  // { namelist: "\\@simple\\;", single: true },
-  // { namelist: "\\@font-face", single: true },
-  // { namelist: "\\@font-face\\;", single: true },
-  // { namelist: "\\031 \\036 px", single: true },
-  // { namelist: "\\031 \\036 px", single: true },
+  { namelist: "simple\\02cinitial", single: true },
+  { namelist: "simple, \\02cinitial" },
+  { namelist: "sim\\020 \\035 ple", single: true },
+  { namelist: "sim\\020 5ple", single: true },
+  { namelist: "\\@simple", single: true },
+  { namelist: "\\@simple\\;", single: true },
+  { namelist: "\\@font-face", single: true },
+  { namelist: "\\@font-face\\;", single: true },
+  { namelist: "\\031 \\036 px", single: true },
+  { namelist: "\\031 \\036 px", single: true },
   { namelist: "\\1f4a9", single: true },
   { namelist: "\\01f4a9", single: true },
   { namelist: "\\0001f4a9", single: true },
@@ -137,7 +128,7 @@ var testFontFamilyLists = [
 
 ];
 
-if (SpecialPowers.getBoolPref("layout.css.unset-value.enabled")) {
+if (window.SpecialPowers && SpecialPowers.getBoolPref("layout.css.unset-value.enabled")) {
   testFontFamilyLists.push(
     { namelist: "unset", invalid: true, fontonly: true, single: true },
     { namelist: "unset, simple", invalid: true },

layout/style/test/test_value_storage.html

@@ -81,21 +81,6 @@ function xfail_compute(property, value)
   return false;
 }
 
-var gBadSerialize = {
-  // Font-families which include quotes don't get those escaped when
-  //serializing: see bug 660397
-  "font-family": ["\\\"Times New Roman", "Times, \\\"Times New Roman"],
-};
-
-function xfail_serialize(property, value)
-{
-  if (property in gBadSerialize &&
-      gBadSerialize[property].indexOf(value) != -1)
-    return true;
-
-  return false;
-}
-
 var gElement = document.getElementById("testnode");
 var gDeclaration = gElement.style;
 var gComputedStyle = window.getComputedStyle(gElement, "");
@@ -171,14 +156,11 @@ function test_property(property)
     gDeclaration.removeProperty(property);
     gDeclaration.setProperty(property, step1val, "");
 
-    var serialize_func = xfail_serialize(property, value) &&
-                         !value_has_variable_reference ? todo_is : is;
-
-    serialize_func(gDeclaration.getPropertyValue(property), step1val,
+    is(gDeclaration.getPropertyValue(property), step1val,
        "parse+serialize should be idempotent for '" +
          property + ": " + value + "'");
     if (test_computed && info.type != CSS_TYPE_TRUE_SHORTHAND) {
-      serialize_func(gComputedStyle.getPropertyValue(property), step1comp,
+      is(gComputedStyle.getPropertyValue(property), step1comp,
          "serialize+parse should be identity transform for '" +
          property + ": " + value + "'");
     }
@@ -215,8 +197,7 @@ function test_property(property)
     if (test_computed && info.type != CSS_TYPE_TRUE_SHORTHAND) {
       gDeclaration.removeProperty(property);
       gDeclaration.setProperty(property, step1comp, "");
-      var func = (xfail_compute(property, value) ||
-        xfail_serialize(property, resolved_value || value)) ? todo_is : is;
+      var func = xfail_compute(property, value) ? todo_is : is;
       func(gComputedStyle.getPropertyValue(property), step1comp,
            "parse+compute+serialize should be idempotent for '" +
            property + ": " + value + "'");