Bug 836329: Fix regression handling 'rem' units in media queries. r=bzbarsky approval-mozilla-aurora=akeybl
authorL. David Baron <dbaron@dbaron.org>
Wed, 13 Feb 2013 11:53:56 -0800
changeset 127522 359388a5aa75988019dce39923432c8f0fd754a0
parent 127521 8ec33ec55ff56b0e46626c49b052d70e4b5e62e7
child 127523 284d0218eb3da80346eca665714342a9606b3b0c
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs836329
milestone20.0a2
Bug 836329: Fix regression handling 'rem' units in media queries. r=bzbarsky approval-mozilla-aurora=akeybl I confirmed that the added tests fail without the patch and pass with the patch.
layout/style/nsRuleNode.cpp
layout/style/test/test_media_queries.html
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -316,37 +316,45 @@ static nscoord CalcLengthWith(const nsCS
     // other things).  And since the font size of the root element
     // changes rarely, we instead handle dynamic changes to the root
     // element's font size by rebuilding all style data in
     // nsCSSFrameConstructor::RestyleElement.
     case eCSSUnit_RootEM: {
       aPresContext->SetUsesRootEMUnits(true);
       nscoord rootFontSize;
 
+      // NOTE: Be very careful with |styleFont|, since we haven't set
+      // aCanStoreInRuleTree to false yet, so we don't want to introduce
+      // any dependencies on aStyleContext's data here.
+      const nsStyleFont *styleFont =
+        aStyleFont ? aStyleFont : aStyleContext->GetStyleFont();
+
       if (aUseProvidedRootEmSize) {
         // We should use the provided aFontSize as the reference length to
         // scale. This only happens when we are calculating font-size or
         // an equivalent (scriptminsize or CalcLengthWithInitialFont) on
         // the root element, in which case aFontSize is already the
         // value we want.
+        if (aFontSize == -1) {
+          // XXX Should this be styleFont->mSize instead to avoid taking
+          // minfontsize prefs into account?
+          aFontSize = styleFont->mFont.size;
+        }
         rootFontSize = aFontSize;
       } else if (aStyleContext && !aStyleContext->GetParent()) {
         // This is the root element (XXX we don't really know this, but
         // nsRuleNode::SetFont makes the same assumption!), so we should
         // use GetStyleFont on this context to get the root element's
         // font size.
-        const nsStyleFont *styleFont =
-          aStyleFont ? aStyleFont : aStyleContext->GetStyleFont();
         rootFontSize = styleFont->mFont.size;
       } else {
         // This is not the root element or we are calculating something other
         // than font size, so rem is relative to the root element's font size.
         nsRefPtr<nsStyleContext> rootStyle;
-        const nsStyleFont *rootStyleFont =
-          aStyleFont ? aStyleFont : aStyleContext->GetStyleFont();
+        const nsStyleFont *rootStyleFont = styleFont;
         Element* docElement = aPresContext->Document()->GetRootElement();
 
         if (docElement) {
           rootStyle = aPresContext->StyleSet()->ResolveStyleFor(docElement,
                                                                 nullptr);
           if (rootStyle) {
             rootStyleFont = rootStyle->GetStyleFont();
           }
--- a/layout/style/test/test_media_queries.html
+++ b/layout/style/test/test_media_queries.html
@@ -228,16 +228,24 @@ function run() {
     should_not_apply("all and (min-" + feature + ": " +
                      (Math.ceil(value/em_size) + 1) + "em)");
     should_apply("all and (min-" + feature + ": " +
                  (Math.floor(value/em_size) - 1) + "em)");
     should_apply("all and (max-" + feature + ": " +
                  (Math.ceil(value/em_size) + 1) + "em)");
     should_not_apply("all and (max-" + feature + ": " +
                      (Math.floor(value/em_size) - 1) + "em)");
+    should_not_apply("all and (min-" + feature + ": " +
+                     (Math.ceil(value/em_size) + 1) + "rem)");
+    should_apply("all and (min-" + feature + ": " +
+                 (Math.floor(value/em_size) - 1) + "rem)");
+    should_apply("all and (max-" + feature + ": " +
+                 (Math.ceil(value/em_size) + 1) + "rem)");
+    should_not_apply("all and (max-" + feature + ": " +
+                     (Math.floor(value/em_size) - 1) + "rem)");
   }
 
   change_state(function() {
     iframe_style.width = "0";
   });
   should_apply("all and (height)");
   should_not_apply("all and (width)");
   change_state(function() {