Bug 1269045 part 3: Stop wrapping placeholder frames in anonymous flex items. r=mats
☠☠ backed out by 7c24f4455420 ☠ ☠
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 27 Oct 2016 18:58:26 -0700
changeset 319775 2162d5c9fb54f3e1b993add441287216534b0b9c
parent 319774 7aa8199183fca08ca7d9b309fa01580affb45da7
child 319776 f57f9ac1435edc2d581c4645fa818ffd665d68f7
push id83250
push userdholbert@mozilla.com
push dateFri, 28 Oct 2016 01:59:30 +0000
treeherdermozilla-inbound@16db55b642a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1269045, 1269046
milestone52.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 1269045 part 3: Stop wrapping placeholder frames in anonymous flex items. r=mats This patch also: * Removes some now-unnecessary code from nsFlexContainerFrame, which was for jumping from wrapped-placeholders to their out-of-flow frames (for DOM comparisons). This code is now unnecessary because placeholders won't be wrapped anymore. * Updates some reftests with abspos content to match the updated spec's expectations, with one marked as "fails" for the time being (until bug 1269046 implements css box alignment, which we need to render that test correctly). MozReview-Commit-ID: 8canWfXk6Kf
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsFlexContainerFrame.cpp
layout/reftests/flexbox/flexbox-position-absolute-1-ref.xhtml
layout/reftests/flexbox/flexbox-position-absolute-2-ref.xhtml
layout/reftests/flexbox/flexbox-position-absolute-2.xhtml
layout/reftests/flexbox/flexbox-position-fixed-1-ref.xhtml
layout/reftests/flexbox/flexbox-position-fixed-2-ref.xhtml
layout/reftests/flexbox/flexbox-position-fixed-2.xhtml
layout/reftests/flexbox/reftest.list
layout/reftests/w3c-css/submitted/flexbox/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -10527,24 +10527,20 @@ void nsCSSFrameConstructor::CreateNeeded
   newItem->mChildItems.SetParentHasNoXBLChildren(true);
   iter.InsertItem(newItem);
 }
 
 #ifdef DEBUG
 static bool
 FrameWantsToBeInAnonymousItem(const nsIAtom* aParentType, const nsIFrame* aFrame)
 {
-  // Note: This needs to match the logic in
-  // nsCSSFrameConstructor::FrameConstructionItem::NeedsAnonFlexOrGridItem()
-  if (aParentType == nsGkAtoms::gridContainerFrame) {
-    return aFrame->IsFrameOfType(nsIFrame::eLineParticipant);
-  }
-  MOZ_ASSERT(aParentType == nsGkAtoms::flexContainerFrame);
-  return aFrame->IsFrameOfType(nsIFrame::eLineParticipant) ||
-         aFrame->GetType() == nsGkAtoms::placeholderFrame;
+  MOZ_ASSERT(aParentType == nsGkAtoms::flexContainerFrame ||
+             aParentType == nsGkAtoms::gridContainerFrame);
+
+  return aFrame->IsFrameOfType(nsIFrame::eLineParticipant);
 }
 #endif
 
 static void
 VerifyGridFlexContainerChildren(nsIFrame* aParentFrame,
                                 const nsFrameList& aChildren)
 {
 #ifdef DEBUG
@@ -12682,27 +12678,16 @@ nsCSSFrameConstructor::FrameConstruction
                           nsIAtom* aContainerType,
                           bool aIsWebkitBox)
 {
   if (mFCData->mBits & FCDATA_IS_LINE_PARTICIPANT) {
     // This will be an inline non-replaced box.
     return true;
   }
 
-  // Bug 874718: Flex containers still wrap placeholders; Grid containers don't.
-  if (aContainerType == nsGkAtoms::flexContainerFrame &&
-      !(mFCData->mBits & FCDATA_DISALLOW_OUT_OF_FLOW) &&
-      aState.GetGeometricParent(mStyleContext->StyleDisplay(), nullptr)) {
-    // We're abspos or fixedpos, which means we'll spawn a placeholder which
-    // we'll need to wrap in an anonymous flex item.  So, we just treat
-    // _this_ frame as if _it_ needs to be wrapped in an anonymous flex item,
-    // and then when we spawn the placeholder, it'll end up in the right spot.
-    return true;
-  }
-
   if (aIsWebkitBox &&
       mStyleContext->StyleDisplay()->IsInlineOutsideStyle()) {
     // In a -webkit-box, all inline-level content gets wrapped in an anon item.
     return true;
   }
 
   return false;
 }
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -1116,20 +1116,18 @@ IsOrderLEQWithDOMFallback(nsIFrame* aFra
 
 
   // Special case:
   // If either frame is for generated content from ::before or ::after, then
   // we can't use nsContentUtils::PositionIsBefore(), since that method won't
   // recognize generated content as being an actual sibling of other nodes.
   // We know where ::before and ::after nodes *effectively* insert in the DOM
   // tree, though (at the beginning & end), so we can just special-case them.
-  nsIAtom* pseudo1 =
-    nsPlaceholderFrame::GetRealFrameFor(aFrame1)->StyleContext()->GetPseudo();
-  nsIAtom* pseudo2 =
-    nsPlaceholderFrame::GetRealFrameFor(aFrame2)->StyleContext()->GetPseudo();
+  nsIAtom* pseudo1 = aFrame1->StyleContext()->GetPseudo();
+  nsIAtom* pseudo2 = aFrame2->StyleContext()->GetPseudo();
 
   if (pseudo1 == nsCSSPseudoElements::before ||
       pseudo2 == nsCSSPseudoElements::after) {
     // frame1 is ::before and/or frame2 is ::after => frame1 is LEQ frame2.
     return true;
   }
   if (pseudo1 == nsCSSPseudoElements::after ||
       pseudo2 == nsCSSPseudoElements::before) {
--- a/layout/reftests/flexbox/flexbox-position-absolute-1-ref.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-absolute-1-ref.xhtml
@@ -59,16 +59,16 @@
            ><div class="a"/><div class="b abspos" style="width: 20px"/></div>
       <!-- Middle child abspos: -->
       <div class="flexbox"
            ><div class="a" style="width: 80px"
           /><div class="inflex abspos"
           /><div class="b" style="width: 120px"/></div>
       <!-- Third child abspos, w/ inflexible items & justify-content: space-around: -->
       <div class="flexbox"
-           ><div class="inflex" style="margin-left: 12px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 24px"
+           ><div class="inflex" style="margin-left: 15px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="inflex" style="margin-left: 30px"
           /><div class="noFlexFn abspos"
-          /><div class="inflex" style="margin-left: 48px"/></div>
+          /><div class="inflex" style="margin-left: 30px"/></div>
     </div>
   </body>
 </html>
--- a/layout/reftests/flexbox/flexbox-position-absolute-2-ref.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-absolute-2-ref.xhtml
@@ -36,38 +36,39 @@
       }
       div.inflex {
         width: 20px;
         height: 100px;
         background: gray;
         display: inline-block;
       }
       div.noFlexFn {
-        width: 15px;
-        height: 15px;
+        width: 16px;
+        height: 16px;
         background: teal;
         display: inline-block;
       }
     </style>
   </head>
   <body>
     <div class="containingBlock">
       <!-- First child abspos: -->
       <div class="flexbox"
            ><div class="a abspos" style="width: 30px"/><div class="b"/></div>
       <!-- Second child abspos: -->
       <div class="flexbox"
-           ><div class="a"/><div class="b abspos" style="width: 20px"/></div>
+           ><div class="a"/><div class="b abspos"
+                                 style="width: 20px; left: 0"/></div>
       <!-- Middle child abspos: -->
       <div class="flexbox"
            ><div class="a" style="width: 80px"
-          /><div class="inflex abspos"
+          /><div class="inflex abspos" style="left: 0"
           /><div class="b" style="width: 120px"/></div>
       <!-- Third child abspos, w/ inflexible items & justify-content: space-around: -->
       <div class="flexbox"
-           ><div class="inflex" style="margin-left: 12px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="noFlexFn abspos" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 48px"/></div>
+           ><div class="inflex" style="margin-left: 15px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="noFlexFn abspos" style="left: 90px"
+          /><div class="inflex" style="margin-left: 30px"/></div>
     </div>
   </body>
 </html>
--- a/layout/reftests/flexbox/flexbox-position-absolute-2.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-absolute-2.xhtml
@@ -42,18 +42,18 @@
       }
       div.inflex {
         flex: none;
         width: 20px;
         height: 100px;
         background: gray;
       }
       div.noFlexFn {
-        width: 15px;
-        height: 15px;
+        width: 16px;
+        height: 16px;
         background: teal;
       }
     </style>
   </head>
   <body>
     <div class="containingBlock">
       <!-- First child abspos: -->
       <div class="flexbox"><div class="a abspos"/><div class="b"/></div>
--- a/layout/reftests/flexbox/flexbox-position-fixed-1-ref.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-fixed-1-ref.xhtml
@@ -59,16 +59,16 @@
            ><div class="a"/><div class="b fixedpos" style="width: 20px"/></div>
       <!-- Middle child fixedpos: -->
       <div class="flexbox"
            ><div class="a" style="width: 80px"
           /><div class="inflex fixedpos"
           /><div class="b" style="width: 120px"/></div>
       <!-- Third child fixedpos, w/ inflexible items & justify-content: space-around: -->
       <div class="flexbox"
-           ><div class="inflex" style="margin-left: 12px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 24px"
+           ><div class="inflex" style="margin-left: 15px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="inflex" style="margin-left: 30px"
           /><div class="noFlexFn fixedpos"
-          /><div class="inflex" style="margin-left: 48px"/></div>
+          /><div class="inflex" style="margin-left: 30px"/></div>
     </div>
   </body>
 </html>
--- a/layout/reftests/flexbox/flexbox-position-fixed-2-ref.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-fixed-2-ref.xhtml
@@ -10,17 +10,17 @@
       div.containingBlock {
         top: 15px;
         left: 20px;
         height: 400px;
         position: absolute;
         border: 2px dashed purple;
       }
       .fixedpos {
-        position: fixed;
+        position: absolute;
         border: 2px dotted black;
       }
       div.flexbox {
         width: 200px;
         height: 100px;
       }
       div.a {
         width: 100%;
@@ -36,38 +36,39 @@
       }
       div.inflex {
         width: 20px;
         height: 100px;
         background: gray;
         display: inline-block;
       }
       div.noFlexFn {
-        width: 15px;
-        height: 15px;
+        width: 16px;
+        height: 16px;
         background: teal;
         display: inline-block;
       }
     </style>
   </head>
   <body>
     <div class="containingBlock">
       <!-- First child fixedpos: -->
       <div class="flexbox"
            ><div class="a fixedpos" style="width: 30px"/><div class="b"/></div>
       <!-- Second child fixedpos: -->
       <div class="flexbox"
-           ><div class="a"/><div class="b fixedpos" style="width: 20px"/></div>
+           ><div class="a"/><div class="b fixedpos"
+                                 style="width: 20px; left: 0"/></div>
       <!-- Middle child fixedpos: -->
       <div class="flexbox"
            ><div class="a" style="width: 80px"
-          /><div class="inflex fixedpos"
+          /><div class="inflex fixedpos" style="left: 0"
           /><div class="b" style="width: 120px"/></div>
       <!-- Third child fixedpos, w/ inflexible items & justify-content: space-around: -->
       <div class="flexbox"
-           ><div class="inflex" style="margin-left: 12px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 24px"
-          /><div class="noFlexFn fixedpos" style="margin-left: 24px"
-          /><div class="inflex" style="margin-left: 48px"/></div>
+           ><div class="inflex" style="margin-left: 15px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="inflex" style="margin-left: 30px"
+          /><div class="noFlexFn fixedpos" style="left: 90px"
+          /><div class="inflex" style="margin-left: 30px"/></div>
     </div>
   </body>
 </html>
--- a/layout/reftests/flexbox/flexbox-position-fixed-2.xhtml
+++ b/layout/reftests/flexbox/flexbox-position-fixed-2.xhtml
@@ -42,18 +42,18 @@
       }
       div.inflex {
         flex: none;
         width: 20px;
         height: 100px;
         background: gray;
       }
       div.noFlexFn {
-        width: 15px;
-        height: 15px;
+        width: 16px;
+        height: 16px;
         background: teal;
       }
     </style>
   </head>
   <body>
     <div class="containingBlock">
       <!-- First child fixedpos: -->
       <div class="flexbox"><div class="a fixedpos"/><div class="b"/></div>
--- a/layout/reftests/flexbox/reftest.list
+++ b/layout/reftests/flexbox/reftest.list
@@ -68,22 +68,22 @@ fuzzy-if(skiaContent,3,10) == flexbox-dy
 == flexbox-float-2a.xhtml  flexbox-float-2-ref.xhtml
 == flexbox-float-2b.xhtml  flexbox-float-2-ref.xhtml
 
 # Tests for the order in which we paint flex items
 fails == flexbox-paint-ordering-3.html  flexbox-paint-ordering-3-ref.html # bug 874718
 
 # Tests for handling of absolutely/fixed/relatively-positioned flex items.
 == flexbox-position-absolute-1.xhtml  flexbox-position-absolute-1-ref.xhtml
-== flexbox-position-absolute-2.xhtml  flexbox-position-absolute-2-ref.xhtml
+fails == flexbox-position-absolute-2.xhtml  flexbox-position-absolute-2-ref.xhtml # bug 1269046
 == flexbox-position-absolute-3.xhtml  flexbox-position-absolute-3-ref.xhtml
 == flexbox-position-absolute-4.xhtml  flexbox-position-absolute-4-ref.xhtml
 == flexbox-position-fixed-3.xhtml     flexbox-position-fixed-3-ref.xhtml
 fuzzy-if(Android,16,400) == flexbox-position-fixed-1.xhtml     flexbox-position-fixed-1-ref.xhtml
-fuzzy-if(Android,16,400) == flexbox-position-fixed-2.xhtml     flexbox-position-fixed-2-ref.xhtml
+fuzzy-if(Android,16,400) fails == flexbox-position-fixed-2.xhtml     flexbox-position-fixed-2-ref.xhtml # bug 1269046
 == flexbox-position-fixed-3.xhtml     flexbox-position-fixed-3-ref.xhtml
 == flexbox-position-fixed-4.xhtml     flexbox-position-fixed-4-ref.xhtml
 
 # Tests for inline content in a flexbox that gets wrapped in an anonymous block
 fails == flexbox-inlinecontent-horiz-1a.xhtml flexbox-inlinecontent-horiz-1-ref.xhtml # reference case rendering is incorrect; bug 858333
 fails == flexbox-inlinecontent-horiz-1b.xhtml flexbox-inlinecontent-horiz-1-ref.xhtml # reference case rendering is incorrect; bug 858333
 == flexbox-inlinecontent-horiz-2.xhtml  flexbox-inlinecontent-horiz-2-ref.xhtml
 == flexbox-inlinecontent-horiz-3a.xhtml flexbox-inlinecontent-horiz-3-ref.xhtml
--- a/layout/reftests/w3c-css/submitted/flexbox/reftest.list
+++ b/layout/reftests/w3c-css/submitted/flexbox/reftest.list
@@ -45,18 +45,18 @@ fuzzy-if(Android,158,32) == flexbox-alig
 == flexbox-baseline-multi-item-horiz-001.html flexbox-baseline-multi-item-horiz-001-ref.html
 == flexbox-baseline-multi-item-vert-001.html flexbox-baseline-multi-item-vert-001-ref.html
 == flexbox-baseline-multi-line-horiz-001.html flexbox-baseline-multi-line-horiz-001-ref.html
 == flexbox-baseline-multi-line-horiz-002.html flexbox-baseline-multi-line-horiz-002-ref.html
 == flexbox-baseline-multi-line-horiz-003.html flexbox-baseline-multi-line-horiz-003-ref.html
 == flexbox-baseline-multi-line-horiz-004.html flexbox-baseline-multi-line-horiz-004-ref.html
 == flexbox-baseline-multi-line-vert-001.html flexbox-baseline-multi-line-vert-001-ref.html
 == flexbox-baseline-multi-line-vert-002.html flexbox-baseline-multi-line-vert-002-ref.html
-fails == flexbox-baseline-single-item-001a.html flexbox-baseline-single-item-001-ref.html # bug 1269045
-fails == flexbox-baseline-single-item-001b.html flexbox-baseline-single-item-001-ref.html # bug 1269045
+== flexbox-baseline-single-item-001a.html flexbox-baseline-single-item-001-ref.html
+== flexbox-baseline-single-item-001b.html flexbox-baseline-single-item-001-ref.html
 
 # Basic tests with with blocks as flex items
 == flexbox-basic-block-horiz-001.xhtml flexbox-basic-block-horiz-001-ref.xhtml
 == flexbox-basic-block-vert-001.xhtml flexbox-basic-block-vert-001-ref.xhtml
 
 # Tests for basic handling of <canvas>/<img>/etc as a flex item
 == flexbox-basic-canvas-horiz-001.xhtml   flexbox-basic-canvas-horiz-001-ref.xhtml
 == flexbox-basic-canvas-vert-001.xhtml    flexbox-basic-canvas-vert-001-ref.xhtml