Bug 877890: In CSS flexbox and XUL box model, sort placeholders using their out-of-flow frame's order/ordinal value. r=bz
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 31 May 2013 22:14:03 -0700
changeset 133691 6eb787f348de2b03d007cb725e85b924b4295b7f
parent 133690 5f23427f2f7967c4696a433652158d6d9d79304b
child 133692 6058a619fc99115f34c914074e6d90d0e34edf57
push id24768
push userphilringnalda@gmail.com
push dateSun, 02 Jun 2013 19:06:53 +0000
treeherdermozilla-central@198e38876f7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs877890
milestone24.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 877890: In CSS flexbox and XUL box model, sort placeholders using their out-of-flow frame's order/ordinal value. r=bz
layout/generic/nsFlexContainerFrame.cpp
layout/reftests/box-ordinal/box-ordinal-with-out-of-flow-1-ref.html
layout/reftests/box-ordinal/box-ordinal-with-out-of-flow-1.html
layout/reftests/box-ordinal/reftest.list
layout/reftests/flexbox/flexbox-paint-ordering-3-ref.html
layout/reftests/flexbox/flexbox-paint-ordering-3.html
layout/reftests/flexbox/reftest.list
layout/xul/base/src/nsBoxFrame.cpp
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -7,16 +7,17 @@
 
 /* rendering object for CSS "display: flex" */
 
 #include "nsFlexContainerFrame.h"
 #include "nsContentUtils.h"
 #include "nsCSSAnonBoxes.h"
 #include "nsDisplayList.h"
 #include "nsLayoutUtils.h"
+#include "nsPlaceholderFrame.h"
 #include "nsPresContext.h"
 #include "nsStyleContext.h"
 #include "prlog.h"
 #include <algorithm>
 
 using namespace mozilla::css;
 using namespace mozilla::layout;
 
@@ -533,21 +534,27 @@ IsOrderLEQWithDOMFallback(nsIFrame* aFra
 
   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;
   }
 
-  int32_t order1 = aFrame1->StylePosition()->mOrder;
-  int32_t order2 = aFrame2->StylePosition()->mOrder;
+  // 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);
 
-  if (order1 != order2) {
-    return order1 < order2;
+    int32_t order1 = aRealFrame1->StylePosition()->mOrder;
+    int32_t order2 = aRealFrame2->StylePosition()->mOrder;
+
+    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
   // the actual frame that corresponds to our child content.
   aFrame1 = GetFirstNonAnonBoxDescendant(aFrame1);
   aFrame2 = GetFirstNonAnonBoxDescendant(aFrame2);
   MOZ_ASSERT(aFrame1 && aFrame2,
@@ -598,18 +605,22 @@ IsOrderLEQWithDOMFallback(nsIFrame* aFra
  */
 bool
 IsOrderLEQ(nsIFrame* aFrame1,
            nsIFrame* aFrame2)
 {
   MOZ_ASSERT(aFrame1->IsFlexItem() && aFrame2->IsFlexItem(),
              "this method only intended for comparing flex items");
 
-  int32_t order1 = aFrame1->StylePosition()->mOrder;
-  int32_t order2 = aFrame2->StylePosition()->mOrder;
+  // 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;
 
   return order1 <= order2;
 }
 
 bool
 nsFlexContainerFrame::IsHorizontal()
 {
   const FlexboxAxisTracker axisTracker(this);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/box-ordinal/box-ordinal-with-out-of-flow-1-ref.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <style>
+    #container { display: -moz-box; }
+    #highOrdinal {
+      background: lime;
+      height: 100px; width: 100px;
+    }
+  </style>
+</head>
+<body>
+  <div id="container">
+    <div id="highOrdinal"></div>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/box-ordinal/box-ordinal-with-out-of-flow-1.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!-- Testcase for whether we honor "-moz-box-ordinal-group" on
+     absolutely-positioned elements within a -moz-box. Specifically,
+     we test whether it affects their paint-order (as it should, since
+     it reorders the frame tree).
+-->
+<html>
+<head>
+  <style>
+    #container { display: -moz-box; }
+    #lowOrdinal {
+      position: absolute;
+      -moz-box-ordinal-group: 5;
+      background: red;
+      height: 100px; width: 100px;
+    }
+    #highOrdinal {
+      position: absolute;
+      -moz-box-ordinal-group: 10;
+      background: lime;
+      height: 100px; width: 100px;
+    }
+    #noOrdinal {
+      position: absolute;
+      background: orange;
+      height: 100px; width: 100px;
+    }
+  </style>
+</head>
+<body>
+  <div id="container">
+    <div id="highOrdinal"></div>
+    <div id="lowOrdinal"></div>
+    <div id="noOrdinal"></div>
+  </div>
+</body>
+</html>
--- a/layout/reftests/box-ordinal/reftest.list
+++ b/layout/reftests/box-ordinal/reftest.list
@@ -1,6 +1,7 @@
+== box-ordinal-with-out-of-flow-1.html box-ordinal-with-out-of-flow-1-ref.html
 == dynamic-1-remove-to-none-grouped.xul dynamic-1-ref.xul
 == dynamic-1-add-to-one-grouped.xul dynamic-1-ref.xul
 == dynamic-1-remove-to-one-grouped-1.xul dynamic-1-ref.xul
 fails == dynamic-1-remove-to-one-grouped-2.xul dynamic-1-ref.xul # bug 575500
 == dynamic-1-add-to-two-grouped-1.xul dynamic-1-ref.xul
 == dynamic-1-add-to-two-grouped-2.xul dynamic-1-ref.xul
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-paint-ordering-3-ref.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <style>
+    #container { display: flex; }
+    #highOrdinal {
+      background: lime;
+      height: 100px; width: 100px;
+    }
+  </style>
+</head>
+<body>
+  <div id="container">
+    <div id="highOrdinal"></div>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-paint-ordering-3.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!-- Testcase for whether we honor "order" on absolutely-positioned elements
+     within a flex container. Specifically, we test whether it affects their
+     paint-order (as it should, since it reorders the frame tree).  -->
+<html>
+<head>
+  <style>
+    #container { display: flex; }
+    #lowOrdinal {
+      position: absolute;
+      order: 5;
+      background: red;
+      height: 100px; width: 100px;
+    }
+    #highOrdinal {
+      position: absolute;
+      order: 10;
+      background: lime;
+      height: 100px; width: 100px;
+    }
+    #noOrdinal {
+      position: absolute;
+      background: orange;
+      height: 100px; width: 100px;
+    }
+  </style>
+</head>
+<body>
+  <div id="container">
+    <div id="highOrdinal"></div>
+    <div id="lowOrdinal"></div>
+    <div id="noOrdinal"></div>
+  </div>
+</body>
+</html>
--- a/layout/reftests/flexbox/reftest.list
+++ b/layout/reftests/flexbox/reftest.list
@@ -133,16 +133,17 @@ fuzzy-if(d2d&&layersGPUAccelerated,24,14
 
 # Tests for "min-width" and "min-height" on flex items.
 == flexbox-minSize-horiz-1.xhtml flexbox-minSize-horiz-1-ref.xhtml
 fails == flexbox-minSize-vert-1.xhtml  flexbox-minSize-vert-1-ref.xhtml # bug 852367
 
 # Tests for the order in which we paint flex items
 == flexbox-paint-ordering-1.xhtml flexbox-paint-ordering-1-ref.xhtml
 == flexbox-paint-ordering-2.xhtml flexbox-paint-ordering-2-ref.xhtml
+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
 == 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
 == flexbox-position-fixed-1.xhtml     flexbox-position-fixed-1-ref.xhtml
--- a/layout/xul/base/src/nsBoxFrame.cpp
+++ b/layout/xul/base/src/nsBoxFrame.cpp
@@ -30,16 +30,17 @@
 // reflow occurs. The preferred size of a child deep in the hierarchy could change. And this could change
 // any number of syblings around the box. Basically any children in the reflow chain must have their caches cleared
 // so when asked for there current size they can relayout themselves. 
 
 #include "nsBoxLayoutState.h"
 #include "nsBoxFrame.h"
 #include "mozilla/dom/Touch.h"
 #include "nsStyleContext.h"
+#include "nsPlaceholderFrame.h"
 #include "nsPresContext.h"
 #include "nsCOMPtr.h"
 #include "nsINameSpaceManager.h"
 #include "nsGkAtoms.h"
 #include "nsIContent.h"
 #include "nsHTMLParts.h"
 #include "nsViewManager.h"
 #include "nsView.h"
@@ -1884,17 +1885,20 @@ nsBoxFrame::SupportsOrdinalsInChildren()
 }
 
 // Helper less-than-or-equal function, used in CheckBoxOrder() as a
 // template-parameter for the sorting functions.
 bool
 IsBoxOrdinalLEQ(nsIFrame* aFrame1,
                 nsIFrame* aFrame2)
 {
-  return aFrame1->GetOrdinal() <= aFrame2->GetOrdinal();
+  // If we've got a placeholder frame, use its out-of-flow frame's ordinal val.
+  nsIFrame* aRealFrame1 = nsPlaceholderFrame::GetRealFrameFor(aFrame1);
+  nsIFrame* aRealFrame2 = nsPlaceholderFrame::GetRealFrameFor(aFrame2);
+  return aRealFrame1->GetOrdinal() <= aRealFrame2->GetOrdinal();
 }
 
 void 
 nsBoxFrame::CheckBoxOrder()
 {
   if (SupportsOrdinalsInChildren() &&
       !nsLayoutUtils::IsFrameListSorted<IsBoxOrdinalLEQ>(mFrames)) {
     nsLayoutUtils::SortFrameList<IsBoxOrdinalLEQ>(mFrames);