Bug 1430616 - correctly find end of keyword table in InspectorUtils; r?heycam draft
authorTom Tromey <tom@tromey.com>
Mon, 15 Jan 2018 09:14:55 -0700
changeset 720471 502c20ecf8437d0767580a8f66e775e7cdc70808
parent 720470 fb4cc60cfca29f7e18949c87f070085f26ead21f
child 720987 aeae054880a805bab8dfddadf70d0d4ad15ee14c
child 720995 717d4b9b291e416986fef4bc147591e9981a1931
push id95556
push userbmo:ttromey@mozilla.com
push dateMon, 15 Jan 2018 17:20:41 +0000
reviewersheycam
bugs1430616
milestone59.0a1
Bug 1430616 - correctly find end of keyword table in InspectorUtils; r?heycam Currently, InspectorUtils::GetCSSValuesForProperty will not return "match-parent" for "text-align". The bug is that InspectorUtils uses an out-of-date approach to finding the end of the keyword table; and this approach conflicts with the special "unsafe" handling in TextAlignUnsafeEnabledPrefChangeCallback: https://dxr.mozilla.org/mozilla-central/rev/21ddfb9e6cc008e47da89db50e22697dc7b38635/layout/base/nsLayoutUtils.cpp#317-321 MozReview-Commit-ID: 58qfKQwIyMX
devtools/shared/css/generated/properties-db.js
layout/inspector/InspectorUtils.cpp
layout/inspector/tests/test_bug877690.html
layout/style/nsCSSProps.cpp
layout/style/nsCSSProps.h
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -8524,16 +8524,17 @@ exports.CSS_PROPERTIES = {
       "-moz-left",
       "-moz-right",
       "center",
       "end",
       "inherit",
       "initial",
       "justify",
       "left",
+      "match-parent",
       "right",
       "start",
       "unset"
     ]
   },
   "text-align-last": {
     "isInherited": true,
     "subproperties": [
--- a/layout/inspector/InspectorUtils.cpp
+++ b/layout/inspector/InspectorUtils.cpp
@@ -415,24 +415,27 @@ static void GetKeywordsForProperty(const
 {
   if (nsCSSProps::IsShorthand(aProperty)) {
     // Shorthand props have no keywords.
     return;
   }
   const nsCSSProps::KTableEntry* keywordTable =
     nsCSSProps::kKeywordTableTable[aProperty];
   if (keywordTable) {
-    for (size_t i = 0; keywordTable[i].mKeyword != eCSSKeyword_UNKNOWN; ++i) {
+    for (size_t i = 0; !keywordTable[i].IsSentinel(); ++i) {
       nsCSSKeyword word = keywordTable[i].mKeyword;
 
       // These are extra -moz values which are added while rebuilding
       // the properties db. These values are not relevant and are not
       // documented on MDN, so filter these out
+      // eCSSKeyword_UNKNOWN is ignored because it indicates an
+      // invalid entry; but can still be seen in a table, see bug 1430616.
       if (word != eCSSKeyword__moz_zoom_in && word != eCSSKeyword__moz_zoom_out &&
-          word != eCSSKeyword__moz_grab && word != eCSSKeyword__moz_grabbing) {
+          word != eCSSKeyword__moz_grab && word != eCSSKeyword__moz_grabbing &&
+          word != eCSSKeyword_UNKNOWN) {
           InsertNoDuplicates(aArray,
                   NS_ConvertASCIItoUTF16(nsCSSKeywords::GetStringValue(word)));
       }
     }
   }
 }
 
 static void GetColorsForProperty(const uint32_t aParserVariant,
--- a/layout/inspector/tests/test_bug877690.html
+++ b/layout/inspector/tests/test_bug877690.html
@@ -224,16 +224,21 @@ function do_test() {
 
   // Regression test for bug 1255384.
   for (prop of ["counter-increment", "counter-reset"]) {
     var values = InspectorUtils.getCSSValuesForProperty(prop);
     var expected = [ "inherit", "initial", "unset", "none" ];
     ok(testValues(values, expected), "property " + prop + "'s values.");
   }
 
+  // Regression test for bug 1430616
+  var prop = "text-align";
+  var values = InspectorUtils.getCSSValuesForProperty(prop);
+  ok(values.includes("match-parent"), "property text-align includes match-parent");
+
   SimpleTest.finish();
 }
 
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(do_test);
 
 </script>
 </head>
--- a/layout/style/nsCSSProps.cpp
+++ b/layout/style/nsCSSProps.cpp
@@ -2429,38 +2429,31 @@ const KTableEntry nsCSSProps::kColumnFil
 };
 
 const KTableEntry nsCSSProps::kColumnSpanKTable[] = {
   { eCSSKeyword_all, NS_STYLE_COLUMN_SPAN_ALL },
   { eCSSKeyword_none, NS_STYLE_COLUMN_SPAN_NONE },
   { eCSSKeyword_UNKNOWN, -1 }
 };
 
-static inline bool
-IsKeyValSentinel(const KTableEntry& aTableEntry)
-{
-  return aTableEntry.mKeyword == eCSSKeyword_UNKNOWN &&
-         aTableEntry.mValue == -1;
-}
-
 int32_t
 nsCSSProps::FindIndexOfKeyword(nsCSSKeyword aKeyword,
                                const KTableEntry aTable[])
 {
   if (eCSSKeyword_UNKNOWN == aKeyword) {
     // NOTE: we can have keyword tables where eCSSKeyword_UNKNOWN is used
     // not only for the sentinel, but also in the middle of the table to
     // knock out values that have been disabled by prefs, e.g. kDisplayKTable.
     // So we deal with eCSSKeyword_UNKNOWN up front to avoid returning a valid
     // index in the loop below.
     return -1;
   }
   for (int32_t i = 0; ; ++i) {
     const KTableEntry& entry = aTable[i];
-    if (::IsKeyValSentinel(entry)) {
+    if (entry.IsSentinel()) {
       break;
     }
     if (aKeyword == entry.mKeyword) {
       return i;
     }
   }
   return -1;
 }
@@ -2481,17 +2474,17 @@ nsCSSKeyword
 nsCSSProps::ValueToKeywordEnum(int32_t aValue, const KTableEntry aTable[])
 {
 #ifdef DEBUG
   typedef decltype(aTable[0].mValue) table_value_type;
   NS_ASSERTION(table_value_type(aValue) == aValue, "Value out of range");
 #endif
   for (int32_t i = 0; ; ++i) {
     const KTableEntry& entry = aTable[i];
-    if (::IsKeyValSentinel(entry)) {
+    if (entry.IsSentinel()) {
       break;
     }
     if (aValue == entry.mValue) {
       return entry.mKeyword;
     }
   }
   return eCSSKeyword_UNKNOWN;
 }
--- a/layout/style/nsCSSProps.h
+++ b/layout/style/nsCSSProps.h
@@ -354,16 +354,21 @@ public:
     constexpr KTableEntry(nsCSSKeyword aKeyword, T aValue)
       : mKeyword(aKeyword)
       , mValue(static_cast<int16_t>(aValue))
     {
       static_assert(mozilla::EnumTypeFitsWithin<T, int16_t>::value,
                     "aValue must be an enum that fits within mValue");
     }
 
+    bool IsSentinel() const
+    {
+      return mKeyword == eCSSKeyword_UNKNOWN && mValue == -1;
+    }
+
     nsCSSKeyword mKeyword;
     int16_t mValue;
   };
 
   static void AddRefTable(void);
   static void ReleaseTable(void);
 
   // Looks up the property with name aProperty and returns its corresponding