Bug 1582819: Add diagnostic assert to catch positioned elements with display:-moz-box or -moz-inline-box that were previously blockified to 'block' but won't be anymore. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Wed, 25 Sep 2019 20:38:55 +0000
changeset 494997 e79b84e42250763cf51680a7314d614ad0e3006f
parent 494996 382a41b6d4767b25c4d61196d865eaa1f6a1d0bb
child 494998 7aa2279f941cf9c5f5fdf4d786604ac7dca053ea
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1582819
milestone71.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 1582819: Add diagnostic assert to catch positioned elements with display:-moz-box or -moz-inline-box that were previously blockified to 'block' but won't be anymore. r=mats Differential Revision: https://phabricator.services.mozilla.com/D46681
layout/base/nsCSSFrameConstructor.cpp
layout/xul/test/test_blockify_moz_box.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -5698,16 +5698,24 @@ bool nsCSSFrameConstructor::AtLineBounda
 }
 
 void nsCSSFrameConstructor::ConstructFramesFromItem(
     nsFrameConstructorState& aState, FCItemIterator& aIter,
     nsContainerFrame* aParentFrame, nsFrameList& aFrameList) {
   FrameConstructionItem& item = aIter.item();
   ComputedStyle* computedStyle = item.mComputedStyle;
 
+  const auto* disp = computedStyle->StyleDisplay();
+  MOZ_DIAGNOSTIC_ASSERT(!disp->IsAbsolutelyPositionedStyle() ||
+                            (disp->mDisplay != StyleDisplay::MozBox &&
+                             disp->mDisplay != StyleDisplay::MozInlineBox),
+                        "This may be a frame that was previously blockified "
+                        "but isn't any longer! It probably needs explicit "
+                        "'display:block' to preserve behavior");
+
   if (item.mIsText) {
     // If this is collapsible whitespace next to a line boundary,
     // don't create a frame. item.IsWhitespace() also sets the
     // NS_CREATE_FRAME_IF_NON_WHITESPACE flag in the text node. (If we
     // end up creating a frame, nsTextFrame::Init will clear the flag.)
     // We don't do this for generated content, because some generated
     // text content is empty text nodes that are about to be initialized.
     // (We check mAdditionalStateBits because only the generated content
--- a/layout/xul/test/test_blockify_moz_box.html
+++ b/layout/xul/test/test_blockify_moz_box.html
@@ -44,19 +44,21 @@ https://bugzilla.mozilla.org/show_bug.cg
   </div>
 
   <!-- Boxes that are directly blockified via other styling on them: -->
   <!-- XXXdholbert commenting these out -- see notes below about assertion
        failures for floated -moz-box.
   <div class="float moz-box" id="floatMozBox"></div>
   <div class="float moz-inline-box" id="floatMozInlineBox"></div>
   -->
+  <!-- XXXdholbert commenting these out -- see notes below about assertion
+       failures for positioned -moz-box.
   <div class="abs moz-box" id="absMozBox"></div>
   <div class="abs moz-inline-box" id="absMozInlineBox"></div>
-
+  -->
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 1580012 **/
 SimpleTest.waitForExplicitFinish();
 
 function checkDisp(elemId, expectedDisplay) {
@@ -95,25 +97,24 @@ function runTest() {
   // ReflowInput.cpp to validate that we never actually encounter a floated
   // -moz-box/-moz-inline-box, and I'm commenting out these lines (which
   // trigger that fatal assertion).
   //
   //  checkDisp("floatMozBox",                "-moz-box");
   //  checkDisp("floatMozInlineBox",          "-moz-box");
 
 
-  // XXXdholbert Similarly, these ones end up with a weird state that I'd like
-  // to forbid, but can't yet because we do actually have some XUL boxes that
-  // are abs/fixed-pos (and depend on being blockified) right now. For now I'm
-  // leaving these uncommented, but as we purge abspos -moz-box content from
-  // the codebase, we can make these trigger a fatal assertion as well, at
-  // which point these lines/elements will need to be removed from this test to
-  // avoid tripping the fatal assertion.
-  checkDisp("absMozBox",                  "-moz-box");
-  checkDisp("absMozInlineBox",            "-moz-box");
+  // XXXdholbert These abspos boxes trigger a diagnostic assertion added in
+  // bug 1582819 which is intended to flush out XUL content that is positioned
+  // and hence was previously blockified to 'block' but will now be '-moz-box'.
+  // The diagnostic assertion doesn't need to stay around forever, but while
+  // it exists, we can't test this scenario without triggering it.
+  //
+  // checkDisp("absMozBox",                  "-moz-box");
+  // checkDisp("absMozInlineBox",            "-moz-box");
 
   SimpleTest.finish();
 }
 
 SpecialPowers.pushPrefEnv({
   "set": [
     ["layout.css.xul-box-display-values.content.enabled", true],
     ["layout.css.xul-box-display-values.survive-blockification.enabled", true]