Bug 1269045 part 3: Stop wrapping placeholder frames in anonymous flex items. r=mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 27 Oct 2016 14:08:47 -0700
changeset 430451 fdca2fc2f3d1179f9c2c6801f634668a840fe741
parent 430450 5fe8fd86ad37e4055026819f633d36a34533f429
child 430452 0d39800ddd34d66df0deeab322e599201812d4a9
push id33838
push userdholbert@mozilla.com
push dateThu, 27 Oct 2016 21:10:05 +0000
reviewersmats
bugs1269045, 1269046
milestone52.0a1
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. * Marks some reftests with abspos content as "fails" for the time being. These tests will be fixed (and probably rewritten a bit) in bug 1269046, which is where we'll implement the correct static position for abspos children of a flex container. MozReview-Commit-ID: 8canWfXk6Kf
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsFlexContainerFrame.cpp
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/reftest.list
+++ b/layout/reftests/flexbox/reftest.list
@@ -67,23 +67,23 @@ fuzzy-if(skiaContent,3,10) == flexbox-dy
 == flexbox-float-1d.xhtml  flexbox-float-1-ref.xhtml
 == 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-1.xhtml  flexbox-position-absolute-1-ref.xhtml # bug 1269046
+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-1.xhtml     flexbox-position-fixed-1-ref.xhtml # bug 1269046
+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