Bug 280443 p4 - properly escape unquoted font family names. r=heycam
authorJohn Daggett <jdaggett@mozilla.com>
Fri, 06 Jun 2014 15:09:24 +0900
changeset 207344 a73b48626bb5c404a6519c43b5c4e5d7829aa5f0
parent 207343 e39cfafa8517ce7810685f6a99fce6fad919d5f4
child 207345 3af2e9f88905ad83af5492fc210c575675386d7f
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs280443
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 280443 p4 - properly escape unquoted font family names. r=heycam
layout/style/nsStyleUtil.cpp
layout/style/test/test_font_family_parsing.html
layout/style/test/test_value_storage.html
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -138,33 +138,59 @@ nsStyleUtil::AppendEscapedCSSIdent(const
         aReturn.Append(char16_t('\\'));
       }
       aReturn.Append(ch);
     }
   }
   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,
   nsAString& aResult)
 {
   const nsTArray<FontFamilyName>& fontlist = aFamilyList.GetFontlist();
   size_t i, len = fontlist.Length();
   for (i = 0; i < len; i++) {
     if (i != 0) {
       aResult.Append(',');
     }
     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);
         break;
       default:
         name.AppendToString(aResult);
     }
   }
--- a/layout/style/test/test_font_family_parsing.html
+++ b/layout/style/test/test_font_family_parsing.html
@@ -50,52 +50,43 @@ var testFontFamilyLists = [
   { namelist: "ick, patooey, 納豆嫌い" },
   { namelist: "納豆嫌い, 納豆大嫌い" },
   { namelist: "納豆嫌い, 納豆大嫌い, 納豆本当に嫌い" },
   { namelist: "納豆嫌い, 納豆大嫌い, 納豆本当に嫌い, 納豆は好みではない" },
   { namelist: "arial, helvetica, sans-serif" },
   { 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 },
 
   /* escapes */
   { 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 },
   { namelist: "\\AbAb", single: true },
 
   /* keywords */
   { namelist: "italic", single: true },
   { namelist: "bold", single: true },
@@ -132,17 +123,17 @@ var testFontFamilyLists = [
   { namelist: "inherit simple", single: true },
   { namelist: "normal simple", single: true },
   { namelist: "caption", single: true }, // these are keywords for the 'font' property but only when in the first position
   { namelist: "icon", single: true },
   { namelist: "menu", single: true }
 
 ];
 
-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 },
     { namelist: "simple, unset", invalid: true },
     { namelist: "simple, unset bongo" },
     { namelist: "simple, bongo unset" },
     { namelist: "simple unset", single: true },
     { namelist: "unset simple", single: true });
--- a/layout/style/test/test_value_storage.html
+++ b/layout/style/test/test_value_storage.html
@@ -76,31 +76,16 @@ function xfail_compute(property, value)
 {
   if (property in gBadCompute &&
       gBadCompute[property].indexOf(value) != -1)
     return true;
 
   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, "");
 
 var gPrereqDeclaration =
   document.getElementById("prereqsheet").sheet.cssRules[0].style;
 
 function test_property(property)
@@ -166,24 +151,21 @@ function test_property(property)
       }
     }
     is(step1ser, expected_serialization,
        "serialization should match property value");
 
     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 + "'");
     }
 
     if ("subproperties" in info &&
         // Using setProperty over subproperties is not sufficient for
         // system fonts, since the shorthand does more than its parts.
         (property != "font" || !(value in gSystemFont)) &&
@@ -210,18 +192,17 @@ function test_property(property)
       is(gDeclaration.getPropertyValue(property), step1val,
          "parse+split+serialize should be idempotent for '" +
          property + ": " + value + "'");
     }
 
     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 + "'");
     }
     if (test_computed && "subproperties" in info) {
       gDeclaration.removeProperty(property);
       for (idx in info.subproperties) {
         var subprop = info.subproperties[idx];