Bug 1167311 - When we unprefix 'display:-webkit-box' on a whitelisted site, only set flag to unprefix '-moz-box' if we're parsing a series of declarations. r=dbaron, a=lizzard
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 22 May 2015 12:47:01 -0700
changeset 266112 9d2ba69b0ff3
parent 266111 3ff03c918bdc
child 266113 928e51389b65
push id4758
push userryanvm@gmail.com
push date2015-05-26 18:35 +0000
treeherdermozilla-beta@928e51389b65 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, lizzard
bugs1167311
milestone39.0
Bug 1167311 - When we unprefix 'display:-webkit-box' on a whitelisted site, only set flag to unprefix '-moz-box' if we're parsing a series of declarations. r=dbaron, a=lizzard
layout/style/nsCSSParser.cpp
layout/style/test/unprefixing_service_iframe.html
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -1201,33 +1201,46 @@ protected:
   // false.
   bool mInFailingSupportsRule : 1;
 
   // True if we will suppress all parse errors (except unexpected EOFs).
   // This is used to prevent errors for declarations inside a failing
   // @supports rule.
   bool mSuppressErrors : 1;
 
-  // True if we've parsed "display: -webkit-box" as "display: flex" in an
-  // earlier declaration within the current block of declarations, as part of
-  // emulating support for certain -webkit-prefixed properties on certain
-  // sites.
-  bool mDidUnprefixWebkitBoxInEarlierDecl; // not :1 so we can use AutoRestore
-
 #ifdef DEBUG
   // True if any parsing of URL values requires a sheet principal to have
   // been passed in the nsCSSScanner constructor.  This is usually the case.
   // It can be set to false, for example, when we create an nsCSSParser solely
   // to parse a property value to test it for syntactic correctness.  When
   // false, an assertion that mSheetPrincipal is non-null is skipped.  Should
   // not be set to false if any nsCSSValues created during parsing can escape
   // out of the parser.
   bool mSheetPrincipalRequired;
 #endif
 
+  // This enum helps us track whether we've unprefixed "display: -webkit-box"
+  // (treating it as "display: flex") in an earlier declaration within a series
+  // of declarations.  (This only impacts behavior when the function
+  // "ShouldUseUnprefixingService()" returns true, and that should only happen
+  // for a short whitelist of origins.)
+  enum WebkitBoxUnprefixState : uint8_t {
+    eNotParsingDecls, // We are *not* currently parsing a sequence of
+                      // CSS declarations. (default state)
+
+    // The next two enum values indicate that we *are* currently parsing a
+    // sequence of declarations (in ParseDeclarations or ParseDeclarationBlock)
+    // and...
+    eHaveNotUnprefixed, // ...we have not unprefixed 'display:-webkit-box' in
+                        // this sequence of CSS declarations.
+    eHaveUnprefixed // ...we *have* unprefixed 'display:-webkit-box' earlier in
+                    // this sequence of CSS declarations.
+  };
+  WebkitBoxUnprefixState mWebkitBoxUnprefixState;
+
   // Stack of rule groups; used for @media and such.
   InfallibleTArray<nsRefPtr<css::GroupRule> > mGroupStack;
 
   // During the parsing of a property (which may be a shorthand), the data
   // are stored in |mTempData|.  (It is needed to ensure that parser
   // errors cause the data to be ignored, and to ensure that a
   // non-'!important' declaration does not override an '!important'
   // one.)
@@ -1294,20 +1307,20 @@ CSSParserImpl::CSSParserImpl()
     mUnsafeRulesEnabled(false),
     mIsChromeOrCertifiedApp(false),
     mViewportUnitsEnabled(true),
     mHTMLMediaMode(false),
     mParsingCompoundProperty(false),
     mInSupportsCondition(false),
     mInFailingSupportsRule(false),
     mSuppressErrors(false),
-    mDidUnprefixWebkitBoxInEarlierDecl(false),
 #ifdef DEBUG
     mSheetPrincipalRequired(true),
 #endif
+    mWebkitBoxUnprefixState(eNotParsingDecls),
     mNextFree(nullptr)
 {
 }
 
 CSSParserImpl::~CSSParserImpl()
 {
   mData.AssertInitialState();
   mTempData.AssertInitialState();
@@ -1526,19 +1539,20 @@ CSSParserImpl::ParseDeclarations(const n
   *aChanged = false;
 
   NS_PRECONDITION(aSheetPrincipal, "Must have principal here!");
 
   nsCSSScanner scanner(aBuffer, 0);
   css::ErrorReporter reporter(scanner, mSheet, mChildLoader, aSheetURI);
   InitScanner(scanner, reporter, aSheetURI, aBaseURI, aSheetPrincipal);
 
-  MOZ_ASSERT(!mDidUnprefixWebkitBoxInEarlierDecl,
-             "Someone forgot to clear the 'did unprefix webkit-box' flag");
-  AutoRestore<bool> autoRestore(mDidUnprefixWebkitBoxInEarlierDecl);
+  MOZ_ASSERT(mWebkitBoxUnprefixState == eNotParsingDecls,
+             "Someone forgot to clear mWebkitBoxUnprefixState!");
+  AutoRestore<WebkitBoxUnprefixState> autoRestore(mWebkitBoxUnprefixState);
+  mWebkitBoxUnprefixState = eHaveNotUnprefixed;
 
   mSection = eCSSSection_General;
 
   mData.AssertInitialState();
   aDeclaration->ClearData();
   // We could check if it was already empty, but...
   *aChanged = true;
 
@@ -6182,19 +6196,20 @@ CSSParserImpl::ParseSelector(nsCSSSelect
   return true;
 }
 
 css::Declaration*
 CSSParserImpl::ParseDeclarationBlock(uint32_t aFlags, nsCSSContextType aContext)
 {
   bool checkForBraces = (aFlags & eParseDeclaration_InBraces) != 0;
 
-  MOZ_ASSERT(!mDidUnprefixWebkitBoxInEarlierDecl,
-             "Someone forgot to clear the 'did unprefix webkit-box' flag");
-  AutoRestore<bool> restorer(mDidUnprefixWebkitBoxInEarlierDecl);
+  MOZ_ASSERT(mWebkitBoxUnprefixState == eNotParsingDecls,
+             "Someone forgot to clear mWebkitBoxUnprefixState!");
+  AutoRestore<WebkitBoxUnprefixState> autoRestore(mWebkitBoxUnprefixState);
+  mWebkitBoxUnprefixState = eHaveNotUnprefixed;
 
   if (checkForBraces) {
     if (!ExpectSymbol('{', true)) {
       REPORT_UNEXPECTED_TOKEN(PEBadDeclBlockStart);
       OUTPUT_ERROR();
       return nullptr;
     }
   }
@@ -6606,28 +6621,31 @@ CSSParserImpl::LookupKeywordPrefixAware(
 
   if (aKeywordTable == nsCSSProps::kDisplayKTable) {
     if (keyword == eCSSKeyword_UNKNOWN &&
         ShouldUseUnprefixingService() &&
         aKeywordStr.EqualsLiteral("-webkit-box")) {
       // Treat "display: -webkit-box" as "display: flex". In simple scenarios,
       // they largely behave the same, as long as we use the CSS Unprefixing
       // Service to also translate the associated properties.
-      mDidUnprefixWebkitBoxInEarlierDecl = true;
+      if (mWebkitBoxUnprefixState == eHaveNotUnprefixed) {
+        mWebkitBoxUnprefixState = eHaveUnprefixed;
+      }
       return eCSSKeyword_flex;
     }
 
     // If we've seen "display: -webkit-box" in an earlier declaration and we
     // tried to unprefix it to emulate support for it, then we have to watch
     // out for later "display: -moz-box" declarations; they're likely just a
     // halfhearted attempt at compatibility, and they actually end up stomping
     // on our emulation of the earlier -webkit-box display-value, via the CSS
     // cascade. To prevent this problem, we also treat "display: -moz-box" as
     // "display: flex" (but only if we unprefixed an earlier "-webkit-box").
-    if (mDidUnprefixWebkitBoxInEarlierDecl && keyword == eCSSKeyword__moz_box) {
+    if (mWebkitBoxUnprefixState == eHaveUnprefixed &&
+        keyword == eCSSKeyword__moz_box) {
       MOZ_ASSERT(ShouldUseUnprefixingService(),
                  "mDidUnprefixWebkitBoxInEarlierDecl should only be set if "
                  "we're using the unprefixing service on this site");
       return eCSSKeyword_flex;
     }
   }
 
   return keyword;
--- a/layout/style/test/unprefixing_service_iframe.html
+++ b/layout/style/test/unprefixing_service_iframe.html
@@ -223,21 +223,45 @@ function testUnprefixingDisabled()
      "'-webkit-box-flex' shouldn't affect computed 'flex-grow' " +
      "when CSS Unprefixing Service is inactive");
 
   let initialDisplay = getComputedStyleWrapper(elem, "display");
   elem.setAttribute("style", "display:-webkit-box");
   is(getComputedStyleWrapper(elem, "display"), initialDisplay,
      "'display:-webkit-box' shouldn't affect computed 'display' " +
      "when CSS Unprefixing Service is inactive");
+
+  elem.style.display = "-webkit-box";
+  is(getComputedStyleWrapper(elem, "display"), initialDisplay,
+     "Setting elem.style.display to '-webkit-box' shouldn't affect computed " +
+     "'display' when CSS Unprefixing Service is inactive");
+}
+
+// Focused test that CSS Unprefixing Service is functioning properly
+// on direct tweaks to elem.style.display:
+function testStyleDisplayDirectly()
+{
+  let elem = document.getElementById("content");
+  elem.style.display = "-webkit-box";
+
+  is(elem.style.display, "flex",
+     "Setting elem.style.display to '-webkit-box' should produce 'flex' " +
+     "in elem.style.display, when CSS Unprefixing Service is active");
+  is(getComputedStyleWrapper(elem, "display"), "flex",
+     "Setting elem.style.display to '-webkit-box' should produce 'flex' " +
+     "in computed style, when CSS Unprefixing Service is active");
+
+  // clean up:
+  elem.style.display = "";
 }
 
 function startTest()
 {
   if (window.location.hash === "#expectEnabled") {
+    testStyleDisplayDirectly();
     gTestcases.forEach(runOneTest);
   } else if (window.location.hash === "#expectDisabled") {
     testUnprefixingDisabled();
   } else {
     ok(false,
        "Need a recognized 'window.location.hash' to indicate expectation. " +
        "Got: '" + window.location.hash + "'");
   }