Bug 1257688 part 3: Change "-webkit-box-ordinal-group" to alias its -moz equivalent, & change nsFlexContainerFrame to respect it in a -webkit-box. r=mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 04 Apr 2016 15:23:06 -0700
changeset 347453 bf1825e1654ab6c2d5c0c766da6b057c4fbf69c7
parent 347452 d738b13da549d98e10d136bf0bebd888ac919519
child 347454 87848bc4863be6175a41d243da36ed509460f652
push id14582
push userdholbert@mozilla.com
push dateMon, 04 Apr 2016 22:23:15 +0000
reviewersmats
bugs1257688
milestone48.0a1
Bug 1257688 part 3: Change "-webkit-box-ordinal-group" to alias its -moz equivalent, & change nsFlexContainerFrame to respect it in a -webkit-box. r=mats MozReview-Commit-ID: DMB8dk0q11T
layout/generic/nsFlexContainerFrame.cpp
layout/style/nsCSSPropAliasList.h
layout/style/test/property_database.js
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -957,16 +957,39 @@ BuildStrutInfoFromCollapsedItems(const F
         aStruts.AppendElement(StrutInfo(itemIdxInContainer,
                                         line->GetLineCrossSize()));
       }
       itemIdxInContainer++;
     }
   }
 }
 
+// Convenience function to get either the "order" or the "box-ordinal-group"
+// property-value for a flex item (depending on whether the container is a
+// modern flex container or a legacy box).
+static int32_t
+GetOrderOrBoxOrdinalGroup(nsIFrame* aFlexItem, bool aIsLegacyBox)
+{
+  if (aIsLegacyBox) {
+    // We'll be using mBoxOrdinal, which has type uint32_t. However, the modern
+    // 'order' property (whose functionality we're co-opting) has type int32_t.
+    // So: if we happen to have a uint32_t value that's greater than INT32_MAX,
+    // we clamp it rather than letting it overflow. Chances are, this is just
+    // an author using BIG_VALUE anyway, so the clamped value should be fine.
+    // (particularly since sufficiently-huge values are busted in Chrome/WebKit
+    // per https://bugs.chromium.org/p/chromium/issues/detail?id=599645 )
+    uint32_t clampedBoxOrdinal = std::min(aFlexItem->StyleXUL()->mBoxOrdinal,
+                                          static_cast<uint32_t>(INT32_MAX));
+    return static_cast<int32_t>(clampedBoxOrdinal);
+  }
+
+  // Normal case: just use modern 'order' property.
+  return aFlexItem->StylePosition()->mOrder;
+}
+
 // Helper-function to find the first non-anonymous-box descendent of aFrame.
 static nsIFrame*
 GetFirstNonAnonBoxDescendant(nsIFrame* aFrame)
 {
   while (aFrame) {
     nsIAtom* pseudoTag = aFrame->StyleContext()->GetPseudo();
 
     // If aFrame isn't an anonymous container, then it'll do.
@@ -1021,31 +1044,36 @@ GetFirstNonAnonBoxDescendant(nsIFrame* a
  *         Otherwise, returns false.
  */
 bool
 IsOrderLEQWithDOMFallback(nsIFrame* aFrame1,
                           nsIFrame* aFrame2)
 {
   MOZ_ASSERT(aFrame1->IsFlexItem() && aFrame2->IsFlexItem(),
              "this method only intended for comparing flex items");
+  MOZ_ASSERT(aFrame1->GetParent() == aFrame2->GetParent(),
+             "this method only intended for comparing siblings");
+  nsStyleContext* parentFrameSC = aFrame1->GetParent()->StyleContext();
+  bool isInLegacyBox = IsLegacyBox(parentFrameSC->StyleDisplay(),
+                                   parentFrameSC);
 
   if (aFrame1 == aFrame2) {
     // Anything is trivially LEQ itself, so we return "true" here... but it's
     // probably bad if we end up actually needing this, so let's assert.
     NS_ERROR("Why are we checking if a frame is LEQ itself?");
     return true;
   }
 
   // If we've got a placeholder frame, use its out-of-flow frame's 'order' val.
   {
     nsIFrame* aRealFrame1 = nsPlaceholderFrame::GetRealFrameFor(aFrame1);
     nsIFrame* aRealFrame2 = nsPlaceholderFrame::GetRealFrameFor(aFrame2);
 
-    int32_t order1 = aRealFrame1->StylePosition()->mOrder;
-    int32_t order2 = aRealFrame2->StylePosition()->mOrder;
+    int32_t order1 = GetOrderOrBoxOrdinalGroup(aRealFrame1, isInLegacyBox);
+    int32_t order2 = GetOrderOrBoxOrdinalGroup(aRealFrame2, isInLegacyBox);
 
     if (order1 != order2) {
       return order1 < order2;
     }
   }
 
   // The "order" values are equal, so we need to fall back on DOM comparison.
   // For that, we need to dig through any anonymous box wrapper frames to find
@@ -1102,23 +1130,28 @@ IsOrderLEQWithDOMFallback(nsIFrame* aFra
  *         equal to that of aFrame2.  Otherwise, returns false.
  */
 bool
 IsOrderLEQ(nsIFrame* aFrame1,
            nsIFrame* aFrame2)
 {
   MOZ_ASSERT(aFrame1->IsFlexItem() && aFrame2->IsFlexItem(),
              "this method only intended for comparing flex items");
+  MOZ_ASSERT(aFrame1->GetParent() == aFrame2->GetParent(),
+             "this method only intended for comparing siblings");
+  nsStyleContext* parentFrameSC = aFrame1->GetParent()->StyleContext();
+  bool isInLegacyBox = IsLegacyBox(parentFrameSC->StyleDisplay(),
+                                   parentFrameSC);
 
   // If we've got a placeholder frame, use its out-of-flow frame's 'order' val.
   nsIFrame* aRealFrame1 = nsPlaceholderFrame::GetRealFrameFor(aFrame1);
   nsIFrame* aRealFrame2 = nsPlaceholderFrame::GetRealFrameFor(aFrame2);
 
-  int32_t order1 = aRealFrame1->StylePosition()->mOrder;
-  int32_t order2 = aRealFrame2->StylePosition()->mOrder;
+  int32_t order1 = GetOrderOrBoxOrdinalGroup(aRealFrame1, isInLegacyBox);
+  int32_t order2 = GetOrderOrBoxOrdinalGroup(aRealFrame2, isInLegacyBox);
 
   return order1 <= order2;
 }
 
 bool
 nsFlexContainerFrame::IsHorizontal()
 {
   const FlexboxAxisTracker axisTracker(StylePosition(), GetWritingMode());
--- a/layout/style/nsCSSPropAliasList.h
+++ b/layout/style/nsCSSPropAliasList.h
@@ -330,17 +330,17 @@ CSS_PROP_ALIAS(-webkit-box-sizing,
 // for those two display values.)
 // XXXdholbert Not all of these are converted yet, but they will be by the
 // end of this patch stack.
 CSS_PROP_ALIAS(-webkit-box-flex,
                flex_grow,
                WebkitBoxFlex,
                WEBKIT_PREFIX_PREF)
 CSS_PROP_ALIAS(-webkit-box-ordinal-group,
-               order,
+               box_ordinal_group,
                WebkitBoxOrdinalGroup,
                WEBKIT_PREFIX_PREF)
 CSS_PROP_ALIAS(-webkit-box-align,
                box_align,
                WebkitBoxAlign,
                WEBKIT_PREFIX_PREF)
 CSS_PROP_ALIAS(-webkit-box-pack,
                box_pack,
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -7197,18 +7197,18 @@ if (IsCSSPropertyPrefEnabled("layout.css
     type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
     alias_for: "flex-grow",
     subproperties: [ "flex-grow" ],
   };
   gCSSProperties["-webkit-box-ordinal-group"] = {
     domProp: "webkitBoxOrdinalGroup",
     inherited: false,
     type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
-    alias_for: "order",
-    subproperties: [ "order" ],
+    alias_for: "-moz-box-ordinal-group",
+    subproperties: [ "-moz-box-ordinal-group" ],
   };
   /* This one is not an alias - it's implemented as a logical property: */
   gCSSProperties["-webkit-box-orient"] = {
     domProp: "webkitBoxOrient",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     logical: true,
     get_computed: webkit_orient_get_computed,