Bug 1269045 part 3: Stop wrapping placeholder frames in anonymous flex items. r=mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 20 Oct 2016 13:25:29 -0700
changeset 427727 d15d1653c466cd81559b3d6ccd24aca5a093783a
parent 427726 5776c3b902f16c148c318d7298e2202427c92853
child 427728 54e4af1f764fc05ae4785274820b87b331c759ad
push id33099
push userdholbert@mozilla.com
push dateThu, 20 Oct 2016 20:25:54 +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
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -10525,24 +10525,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
@@ -12679,27 +12675,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
@@ -1102,20 +1102,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,25 +67,25 @@ 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
+fails == flexbox-position-absolute-4.xhtml  flexbox-position-absolute-4-ref.xhtml # bug 1269046
 == 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
+fails == flexbox-position-fixed-4.xhtml     flexbox-position-fixed-4-ref.xhtml # bug 1269046
 
 # 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
 == flexbox-inlinecontent-horiz-3b.xhtml flexbox-inlinecontent-horiz-3-ref.xhtml
 == flexbox-inlinecontent-horiz-3c.xhtml flexbox-inlinecontent-horiz-3-ref.xhtml