Bug 851379 part 2: Make ComputeMargin, ComputePadding, and InitOffsets take a horizontal *and* vertical percent basis, so that we can resolve vertical margins and padding against containing block *height* in flex items. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 11 Apr 2013 07:51:52 -0700
changeset 128448 0e4434a4e7f3927ade2d551c2da024eaceb9994f
parent 128447 2cb63e38eeaf5b7f164dd77b3902520107224254
child 128449 8de3dbbec45df06e7b8ca7b633d066a6f81a6942
push id24528
push userryanvm@gmail.com
push dateThu, 11 Apr 2013 19:19:41 +0000
treeherdermozilla-central@7b8ed29c6bc0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs851379
milestone23.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 851379 part 2: Make ComputeMargin, ComputePadding, and InitOffsets take a horizontal *and* vertical percent basis, so that we can resolve vertical margins and padding against containing block *height* in flex items. r=mats
layout/generic/nsHTMLReflowState.cpp
layout/generic/nsHTMLReflowState.h
--- a/layout/generic/nsHTMLReflowState.cpp
+++ b/layout/generic/nsHTMLReflowState.cpp
@@ -1834,18 +1834,27 @@ nsHTMLReflowState::InitConstraints(nsPre
 {
   DISPLAY_INIT_CONSTRAINTS(frame, this,
                            aContainingBlockWidth, aContainingBlockHeight,
                            aBorder, aPadding);
 
   // If this is the root frame, then set the computed width and
   // height equal to the available space
   if (nullptr == parentReflowState) {
+    MOZ_ASSERT(!frame->IsFlexItem(),
+               "the root frame can't be a flex item, since being a flex item "
+               "requires that you have a parent");
+    // Note that we pass the containing block width as the percent basis for
+    // both horizontal *and* vertical margins & padding, in our InitOffsets
+    // call here. This is correct per CSS 2.1; it'd be incorrect for e.g. flex
+    // items and grid items, but the root frame can't be either of those.
     // XXXldb This doesn't mean what it used to!
-    InitOffsets(aContainingBlockWidth, aFrameType, aBorder, aPadding);
+    InitOffsets(aContainingBlockWidth,
+                aContainingBlockWidth,
+                aFrameType, aBorder, aPadding);
     // Override mComputedMargin since reflow roots start from the
     // frame's boundary, which is inside the margin.
     mComputedMargin.SizeTo(0, 0, 0, 0);
     mComputedOffsets.SizeTo(0, 0, 0, 0);
 
     mComputedWidth = availableWidth - mComputedBorderPadding.LeftRight();
     if (mComputedWidth < 0)
       mComputedWidth = 0;
@@ -1882,17 +1891,30 @@ nsHTMLReflowState::InitConstraints(nsPre
         fType = cbrs->frame->GetType();
         if (IS_TABLE_CELL(fType)) {
           // use the cell's computed height 
           aContainingBlockHeight = cbrs->mComputedHeight;
         }
       }
     }
 
-    InitOffsets(aContainingBlockWidth, aFrameType, aBorder, aPadding);
+    // Flex containers resolve percentage margin & padding against the flex
+    // container's height (which is the containing block height).
+    // For everything else: the CSS21 spec requires that margin and padding
+    // percentage values are calculated with respect to the *width* of the
+    // containing block, even for margin & padding in the vertical axis.
+    // XXX Might need to also pass the CB height (not width) for page boxes,
+    // too, if we implement them.
+    nscoord verticalPercentBasis = aContainingBlockWidth;
+    if (frame->IsFlexItem()) {
+      verticalPercentBasis =
+        aContainingBlockHeight == NS_AUTOHEIGHT ? 0 : aContainingBlockHeight;
+    }
+    InitOffsets(aContainingBlockWidth, verticalPercentBasis,
+                aFrameType, aBorder, aPadding);
 
     const nsStyleCoord &height = mStylePosition->mHeight;
     nsStyleUnit heightUnit = height.GetUnit();
 
     // Check for a percentage based height and a containing block height
     // that depends on the content height
     // XXX twiddling heightUnit doesn't help anymore
     // FIXME Shouldn't we fix that?
@@ -2105,34 +2127,37 @@ UpdateProp(FrameProperties& aProps,
       aProps.Set(aProperty, new nsMargin(aNewValue));
     }
   } else {
     aProps.Delete(aProperty);
   }
 }
 
 void
-nsCSSOffsetState::InitOffsets(nscoord aContainingBlockWidth,
+nsCSSOffsetState::InitOffsets(nscoord aHorizontalPercentBasis,
+                              nscoord aVerticalPercentBasis,
                               nsIAtom* aFrameType,
                               const nsMargin *aBorder,
                               const nsMargin *aPadding)
 {
-  DISPLAY_INIT_OFFSETS(frame, this, aContainingBlockWidth, aBorder, aPadding);
+  // XXXdholbert This macro probably needs to take aVerticalPercentBasis too
+  DISPLAY_INIT_OFFSETS(frame, this, aHorizontalPercentBasis, aBorder, aPadding);
 
   // Since we are in reflow, we don't need to store these properties anymore
   // unless they are dependent on width, in which case we store the new value.
   nsPresContext *presContext = frame->PresContext();
   FrameProperties props(presContext->PropertyTable(), frame);
   props.Delete(nsIFrame::UsedBorderProperty());
 
   // Compute margins from the specified margin style information. These
   // become the default computed values, and may be adjusted below
   // XXX fix to provide 0,0 for the top&bottom margins for
   // inline-non-replaced elements
-  bool needMarginProp = ComputeMargin(aContainingBlockWidth);
+  bool needMarginProp = ComputeMargin(aHorizontalPercentBasis,
+                                      aVerticalPercentBasis);
   // XXX We need to include 'auto' horizontal margins in this too!
   // ... but if we did that, we'd need to fix nsFrame::GetUsedMargin
   // to use it even when the margins are all zero (since sometimes
   // they get treated as auto)
   ::UpdateProp(props, nsIFrame::UsedMarginProperty(), needMarginProp,
                mComputedMargin);
 
 
@@ -2154,17 +2179,18 @@ nsCSSOffsetState::InitOffsets(nscoord aC
     mComputedPadding.SizeTo(0, 0, 0, 0);
     needPaddingProp = false;
   }
   else if (aPadding) { // padding is an input arg
     mComputedPadding = *aPadding;
     needPaddingProp = frame->StylePadding()->IsWidthDependent();
   }
   else {
-    needPaddingProp = ComputePadding(aContainingBlockWidth, aFrameType);
+    needPaddingProp = ComputePadding(aHorizontalPercentBasis,
+                                     aVerticalPercentBasis, aFrameType);
   }
 
   if (isThemed) {
     nsIntMargin widget;
     presContext->GetTheme()->GetWidgetBorder(presContext->DeviceContext(),
                                              frame, disp->mAppearance,
                                              &widget);
     mComputedBorderPadding.top =
@@ -2414,95 +2440,92 @@ nsHTMLReflowState::CalcLineHeight(nsStyl
     ComputeLineHeight(aStyleContext, aBlockHeight, aFontSizeInflation);
 
   NS_ASSERTION(lineHeight >= 0, "ComputeLineHeight screwed up");
 
   return lineHeight;
 }
 
 bool
-nsCSSOffsetState::ComputeMargin(nscoord aContainingBlockWidth)
+nsCSSOffsetState::ComputeMargin(nscoord aHorizontalPercentBasis,
+                                nscoord aVerticalPercentBasis)
 {
   // SVG text frames have no margin.
   if (frame->IsSVGText()) {
     return false;
   }
 
   // If style style can provide us the margin directly, then use it.
   const nsStyleMargin *styleMargin = frame->StyleMargin();
-  bool isWidthDependent = !styleMargin->GetMargin(mComputedMargin);
-  if (isWidthDependent) {
+  bool isCBDependent = !styleMargin->GetMargin(mComputedMargin);
+  if (isCBDependent) {
     // We have to compute the value
     mComputedMargin.left = nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aHorizontalPercentBasis,
                               styleMargin->mMargin.GetLeft());
     mComputedMargin.right = nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aHorizontalPercentBasis,
                               styleMargin->mMargin.GetRight());
 
-    // According to the CSS2 spec, margin percentages are
-    // calculated with respect to the *width* of the containing
-    // block, even for margin-top and margin-bottom.
-    // XXX This isn't true for page boxes, if we implement them.
     mComputedMargin.top = nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aVerticalPercentBasis,
                               styleMargin->mMargin.GetTop());
     mComputedMargin.bottom = nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aVerticalPercentBasis,
                               styleMargin->mMargin.GetBottom());
   }
 
   nscoord marginAdjustment = FontSizeInflationListMarginAdjustment(frame);
 
   if (marginAdjustment > 0) {
     const nsStyleVisibility* visibility = frame->StyleVisibility();
     if (visibility->mDirection == NS_STYLE_DIRECTION_RTL) {
       mComputedMargin.right = mComputedMargin.right + marginAdjustment;
     } else {
       mComputedMargin.left = mComputedMargin.left + marginAdjustment;
     }
   }
 
-  return isWidthDependent;
+  return isCBDependent;
 }
 
 bool
-nsCSSOffsetState::ComputePadding(nscoord aContainingBlockWidth, nsIAtom* aFrameType)
+nsCSSOffsetState::ComputePadding(nscoord aHorizontalPercentBasis,
+                                 nscoord aVerticalPercentBasis,
+                                 nsIAtom* aFrameType)
 {
   // If style can provide us the padding directly, then use it.
   const nsStylePadding *stylePadding = frame->StylePadding();
-  bool isWidthDependent = !stylePadding->GetPadding(mComputedPadding);
+  bool isCBDependent = !stylePadding->GetPadding(mComputedPadding);
   // a table row/col group, row/col doesn't have padding
   // XXXldb Neither do border-collapse tables.
   if (nsGkAtoms::tableRowGroupFrame == aFrameType ||
       nsGkAtoms::tableColGroupFrame == aFrameType ||
       nsGkAtoms::tableRowFrame      == aFrameType ||
       nsGkAtoms::tableColFrame      == aFrameType) {
     mComputedPadding.SizeTo(0,0,0,0);
   }
-  else if (isWidthDependent) {
+  else if (isCBDependent) {
     // We have to compute the value
     // clamp negative calc() results to 0
     mComputedPadding.left = std::max(0, nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aHorizontalPercentBasis,
                               stylePadding->mPadding.GetLeft()));
     mComputedPadding.right = std::max(0, nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aHorizontalPercentBasis,
                               stylePadding->mPadding.GetRight()));
 
-    // According to the CSS2 spec, percentages are calculated with respect to
-    // containing block width for padding-top and padding-bottom
     mComputedPadding.top = std::max(0, nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aVerticalPercentBasis,
                               stylePadding->mPadding.GetTop()));
     mComputedPadding.bottom = std::max(0, nsLayoutUtils::
-      ComputeCBDependentValue(aContainingBlockWidth,
+      ComputeCBDependentValue(aVerticalPercentBasis,
                               stylePadding->mPadding.GetBottom()));
   }
-  return isWidthDependent;
+  return isCBDependent;
 }
 
 void
 nsHTMLReflowState::ComputeMinMaxValues(nscoord aContainingBlockWidth,
                                        nscoord aContainingBlockHeight,
                                        const nsHTMLReflowState* aContainingBlockRS)
 {
   mComputedMinWidth = ComputeWidthValue(aContainingBlockWidth,
--- a/layout/generic/nsHTMLReflowState.h
+++ b/layout/generic/nsHTMLReflowState.h
@@ -7,16 +7,17 @@
 
 #ifndef nsHTMLReflowState_h___
 #define nsHTMLReflowState_h___
 
 #include "nsMargin.h"
 #include "nsStyleCoord.h"
 #include "nsStyleStructInlines.h"
 #include "nsIFrame.h"
+#include "mozilla/Assertions.h"
 #include <algorithm>
 
 class nsPresContext;
 class nsRenderingContext;
 class nsFloatManager;
 class nsLineLayout;
 class nsIPercentHeightObserver;
 
@@ -139,22 +140,29 @@ public:
 
   // Callers using this constructor must call InitOffsets on their own.
   nsCSSOffsetState(nsIFrame *aFrame, nsRenderingContext *aRenderingContext)
     : frame(aFrame)
     , rendContext(aRenderingContext)
   {
   }
 
+  // NOTE: If we ever want to use nsCSSOffsetState for a flex item or a grid
+  // item, we need to make it take the containing-block height as well as the
+  // width, since flex items and grid items resolve vertical percent margins
+  // and padding against the containing-block height, rather than its width.
   nsCSSOffsetState(nsIFrame *aFrame, nsRenderingContext *aRenderingContext,
                    nscoord aContainingBlockWidth)
     : frame(aFrame)
     , rendContext(aRenderingContext)
   {
-    InitOffsets(aContainingBlockWidth, frame->GetType());
+    MOZ_ASSERT(!aFrame->IsFlexItem(),
+               "We're about to resolve vertical percent margin & padding "
+               "values against CB width, which is incorrect for flex items");
+    InitOffsets(aContainingBlockWidth, aContainingBlockWidth, frame->GetType());
   }
 
 #ifdef DEBUG
   // Reflow trace methods.  Defined in nsFrame.cpp so they have access
   // to the display-reflow infrastructure.
   static void* DisplayInitOffsetsEnter(nsIFrame* aFrame,
                                        nsCSSOffsetState* aState,
                                        nscoord aCBWidth,
@@ -164,30 +172,49 @@ public:
                                      nsCSSOffsetState* aState,
                                      void* aValue);
 #endif
 
 private:
   /**
    * Computes margin values from the specified margin style information, and
    * fills in the mComputedMargin member.
-   * @return true if the margin is dependent on the containing block width
+   *
+   * @param aHorizontalPercentBasis
+   *    Length to use for resolving percentage margin values in the horizontal
+   *    axis. Usually the containing block width.
+   * @param aVerticalPercentBasis
+   *    Length to use for resolving percentage margin values in the vertical
+   *    axis.  Usually the containing block width, per CSS21 sec 8.3, but may
+   *    be the containing block *height*, e.g. in CSS3 Flexbox and Grid.
+   * @return true if the margin is dependent on the containing block size.
    */
-  bool ComputeMargin(nscoord aContainingBlockWidth);
+  bool ComputeMargin(nscoord aHorizontalPercentBasis,
+                     nscoord aVerticalPercentBasis);
   
   /**
    * Computes padding values from the specified padding style information, and
    * fills in the mComputedPadding member.
-   * @return true if the padding is dependent on the containing block width
+   *
+   * @param aHorizontalPercentBasis
+   *    Length to use for resolving percentage padding values in the horizontal
+   *    axis. Usually the containing block width.
+   * @param aVerticalPercentBasis
+   *    Length to use for resolving percentage padding values in the vertical
+   *    axis.  Usually the containing block width, per CSS21 sec 8.4, but may
+   *    be the containing block *height* in e.g. CSS3 Flexbox and Grid.
+   * @return true if the padding is dependent on the containing block size.
    */
-   bool ComputePadding(nscoord aContainingBlockWidth, nsIAtom* aFrameType);
+   bool ComputePadding(nscoord aHorizontalPercentBasis,
+                       nscoord aVerticalPercentBasis, nsIAtom* aFrameType);
 
 protected:
 
-  void InitOffsets(nscoord aContainingBlockWidth,
+  void InitOffsets(nscoord aHorizontalPercentBasis,
+                   nscoord aVerticalPercentBasis,
                    nsIAtom* aFrameType,
                    const nsMargin *aBorder = nullptr,
                    const nsMargin *aPadding = nullptr);
 
   /*
    * Convert nsStyleCoord to nscoord when percentages depend on the
    * containing block width, and enumerated values are for width,
    * min-width, or max-width.  Does not handle auto widths.