Bug 1427292: Update display: contents on unusual elements to the spec. r?mats draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 28 Dec 2017 19:48:14 +0100
changeset 715216 98b7e598feafa0938ef262463d86f9aa01ec9c2d
parent 715215 626482dbde596a24362612914cb7e13410ea4aef
child 715217 4550665b80bb393fc1bd62a282b72aa80467bd9d
child 715232 82b7c11001bca72a4c50b917bbd4629c8dd6fc2c
push id94095
push userbmo:emilio@crisal.io
push dateWed, 03 Jan 2018 05:45:02 +0000
reviewersmats
bugs1427292
milestone59.0a1
Bug 1427292: Update display: contents on unusual elements to the spec. r?mats This will pass[1] whenever the next WPT sync happens. [1]: https://github.com/w3c/web-platform-tests/blob/master/css/css-display/display-contents-unusual-html-elements-none.html MozReview-Commit-ID: 19dqDSxVm7A
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/reftests/css-display/display-contents-acid-ref.html
layout/reftests/css-display/display-contents-acid.html
layout/reftests/css-display/display-contents-fieldset-ref.html
layout/svg/nsSVGUseFrame.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3618,23 +3618,27 @@ nsCSSFrameConstructor::FindDataByInt(int
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindDataByTag(nsAtom* aTag,
                                      Element* aElement,
                                      nsStyleContext* aStyleContext,
                                      const FrameConstructionDataByTag* aDataPtr,
-                                     uint32_t aDataLength)
+                                     uint32_t aDataLength,
+                                     bool* aFound)
 {
   for (const FrameConstructionDataByTag *curData = aDataPtr,
          *endData = aDataPtr + aDataLength;
        curData != endData;
        ++curData) {
     if (*curData->mTag == aTag) {
+      if (aFound) {
+        *aFound = true;
+      }
       const FrameConstructionData* data = &curData->mData;
       if (data->mBits & FCDATA_FUNC_IS_DATA_GETTER) {
         return data->mFunc.mDataGetter(aElement, aStyleContext);
       }
 
       return data;
     }
   }
@@ -3728,18 +3732,39 @@ nsCSSFrameConstructor::FindHTMLData(Elem
     SIMPLE_TAG_CHAIN(canvas, nsCSSFrameConstructor::FindCanvasData),
     SIMPLE_TAG_CREATE(video, NS_NewHTMLVideoFrame),
     SIMPLE_TAG_CREATE(audio, NS_NewHTMLVideoFrame),
     SIMPLE_TAG_CREATE(progress, NS_NewProgressFrame),
     SIMPLE_TAG_CREATE(meter, NS_NewMeterFrame),
     COMPLEX_TAG_CREATE(details, &nsCSSFrameConstructor::ConstructDetailsFrame)
   };
 
-  return FindDataByTag(aTag, aElement, aStyleContext, sHTMLData,
-                       ArrayLength(sHTMLData));
+  bool found = false;
+  const FrameConstructionData* data =
+    FindDataByTag(aTag, aElement, aStyleContext, sHTMLData,
+                  ArrayLength(sHTMLData), &found);
+
+  // https://drafts.csswg.org/css-display/#unbox-html
+  if (found &&
+      MOZ_UNLIKELY(aStyleContext->StyleDisplay()->mDisplay ==
+                   StyleDisplay::Contents)) {
+    // <button>, <legend>, <details> and <fieldset> don’t have any special
+    // behavior; display: contents simply removes their principal box, and their
+    // contents render as normal.
+    if (aTag != nsGkAtoms::button &&
+        aTag != nsGkAtoms::legend &&
+        aTag != nsGkAtoms::details &&
+        aTag != nsGkAtoms::fieldset) {
+      // On the rest of unusual HTML elements, display: contents creates no box.
+      static const FrameConstructionData sSuppressData = SUPPRESS_FCDATA();
+      return &sSuppressData;
+    }
+  }
+
+  return data;
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindImgData(Element* aElement,
                                    nsStyleContext* aStyleContext)
 {
   if (!nsImageFrame::ShouldCreateImageFrameFor(aElement, aStyleContext)) {
@@ -4492,18 +4517,31 @@ nsCSSFrameConstructor::FindXULTagData(El
                      nsCSSFrameConstructor::FindXULListBoxBodyData),
     SIMPLE_TAG_CHAIN(listitem, nsCSSFrameConstructor::FindXULListItemData),
 #endif /* MOZ_XUL */
     SIMPLE_XUL_CREATE(slider, NS_NewSliderFrame),
     SIMPLE_XUL_CREATE(scrollbar, NS_NewScrollbarFrame),
     SIMPLE_XUL_CREATE(scrollbarbutton, NS_NewScrollbarButtonFrame)
 };
 
-  return FindDataByTag(aTag, aElement, aStyleContext, sXULTagData,
-                       ArrayLength(sXULTagData));
+  bool found = false;
+  const FrameConstructionData* data =
+    FindDataByTag(aTag, aElement, aStyleContext, sXULTagData,
+                  ArrayLength(sXULTagData), &found);
+
+  // There's no spec that says what to do for XUL, but we do the same than for
+  // HTML for consistency.
+  if (found &&
+      MOZ_UNLIKELY(aStyleContext->StyleDisplay()->mDisplay ==
+                   StyleDisplay::Contents)) {
+    static const FrameConstructionData sSuppressData = SUPPRESS_FCDATA();
+    return &sSuppressData;
+  }
+
+  return data;
 }
 
 #ifdef MOZ_XUL
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindPopupGroupData(Element* aElement,
                                           nsStyleContext* /* unused */)
 {
@@ -4966,18 +5004,17 @@ nsCSSFrameConstructor::FindDisplayData(c
                   NS_NewRubyBaseContainerFrame)),
     FCDATA_FOR_DISPLAY(StyleDisplay::RubyText,
       FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT |
                   FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRubyTextContainer),
                   NS_NewRubyTextFrame)),
     FCDATA_FOR_DISPLAY(StyleDisplay::RubyTextContainer,
       FCDATA_DECL(FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRuby),
                   NS_NewRubyTextContainerFrame)),
-    FCDATA_FOR_DISPLAY(StyleDisplay::Contents,
-      FULL_CTOR_FCDATA(FCDATA_IS_CONTENTS, nullptr/*never called*/)),
+    FCDATA_FOR_DISPLAY(StyleDisplay::Contents, UNREACHABLE_FCDATA()),
     FCDATA_FOR_DISPLAY(StyleDisplay::WebkitBox,
       FCDATA_DECL(FCDATA_MAY_NEED_SCROLLFRAME, NS_NewFlexContainerFrame)),
     FCDATA_FOR_DISPLAY(StyleDisplay::WebkitInlineBox,
       FCDATA_DECL(FCDATA_MAY_NEED_SCROLLFRAME, NS_NewFlexContainerFrame)),
     FCDATA_FOR_DISPLAY(StyleDisplay::MozBox,
       FCDATA_DECL(FCDATA_MAY_NEED_SCROLLFRAME, NS_NewFlexContainerFrame)),
     FCDATA_FOR_DISPLAY(StyleDisplay::MozInlineBox,
       FCDATA_DECL(FCDATA_MAY_NEED_SCROLLFRAME, NS_NewFlexContainerFrame)),
@@ -5552,16 +5589,33 @@ nsCSSFrameConstructor::FindSVGData(Eleme
     return &sSuppressData;
   }
 
   // We don't need frames for animation elements
   if (aElement->IsNodeOfType(nsINode::eANIMATION)) {
     return &sSuppressData;
   }
 
+  // https://drafts.csswg.org/css-display/#unbox-svg
+  if (aStyleContext->StyleDisplay()->mDisplay == StyleDisplay::Contents) {
+    // For root <svg> elements, display: contents behaves as display: none.
+    if (aTag == nsGkAtoms::svg && !parentIsSVG) {
+      return &sSuppressData;
+    }
+
+    // For nested <svg>, <g>, <use> and <tspan> behave normally, but any other
+    // element behaves as display: none as well.
+    if (aTag != nsGkAtoms::g &&
+        aTag != nsGkAtoms::use &&
+        aTag != nsGkAtoms::svg &&
+        aTag != nsGkAtoms::tspan) {
+      return &sSuppressData;
+    }
+  }
+
   if (aTag == nsGkAtoms::svg && !parentIsSVG) {
     // We need outer <svg> elements to have an nsSVGOuterSVGFrame regardless
     // of whether they fail conditional processing attributes, since various
     // SVG frames assume that one exists.  We handle the non-rendering
     // of failing outer <svg> element contents like <switch> statements,
     // and do the PassesConditionalProcessingTests call in
     // nsSVGOuterSVGFrame::Init.
     static const FrameConstructionData sOuterSVGData =
@@ -6070,17 +6124,17 @@ nsCSSFrameConstructor::AddFrameConstruct
     !display->IsAbsolutelyPositionedStyle() &&
     !(aParentFrame && aParentFrame->IsGridContainerFrame()) &&
     !(bits & FCDATA_IS_TABLE_PART) && !(bits & FCDATA_IS_SVG_TEXT);
 
   if (canHavePageBreak && display->mBreakBefore) {
     AddPageBreakItem(aContent, aItems);
   }
 
-  if (MOZ_UNLIKELY(bits & FCDATA_IS_CONTENTS)) {
+  if (display->mDisplay == StyleDisplay::Contents) {
     if (!GetDisplayContentsStyleFor(aContent)) {
       MOZ_ASSERT(styleContext->GetPseudo() || !isGeneratedContent,
                  "Should have had pseudo type");
       aState.mFrameManager->RegisterDisplayContentsStyleFor(aContent,
                                                             styleContext);
     } else {
       aState.mFrameManager->ChangeRegisteredDisplayContentsStyleFor(aContent,
                                                                     styleContext);
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -732,20 +732,16 @@ private:
      block formatting context wrapper around the kids of this frame
      using the FrameConstructionData's mPseudoAtom for its anonymous
      box type. */
 #define FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS 0x40000
   /* If FCDATA_IS_SVG_TEXT is set, then this text frame is a descendant of
      an SVG text frame. */
 #define FCDATA_IS_SVG_TEXT 0x80000
   /**
-   * display:contents
-   */
-#define FCDATA_IS_CONTENTS 0x100000
-  /**
    * When FCDATA_CREATE_BLOCK_WRAPPER_FOR_ALL_KIDS is set, this bit says
    * if we should create a grid/flex/columnset container instead of
    * a block wrapper when the styles says so.
    */
 #define FCDATA_ALLOW_GRID_FLEX_COLUMNSET 0x200000
   /**
    * Whether the kids of this FrameConstructionData should be flagged as having
    * a wrapper anon box parent.  This should only be set if
@@ -825,26 +821,31 @@ private:
      match or if the matching integer has a FrameConstructionDataGetter that
      returns null. */
   static const FrameConstructionData*
     FindDataByInt(int32_t aInt, Element* aElement,
                   nsStyleContext* aStyleContext,
                   const FrameConstructionDataByInt* aDataPtr,
                   uint32_t aDataLength);
 
-  /* A function that takes a tag, content, style context, and array of
-     FrameConstructionDataByTags and finds the appropriate frame construction
-     data to use and returns it.  This can return null if none of the tags
-     match or if the matching tag has a FrameConstructionDataGetter that
-     returns null. */
+  /**
+   * A function that takes a tag, content, style context, and array of
+   * FrameConstructionDataByTags and finds the appropriate frame construction
+   * data to use and returns it.
+   *
+   * This can return null if none of the tags match or if the matching tag has a
+   * FrameConstructionDataGetter that returns null. In the case that the tags
+   * actually match, aFound will be true, even if the return value is null.
+   */
   static const FrameConstructionData*
     FindDataByTag(nsAtom* aTag, Element* aElement,
                   nsStyleContext* aStyleContext,
                   const FrameConstructionDataByTag* aDataPtr,
-                  uint32_t aDataLength);
+                  uint32_t aDataLength,
+                  bool* aFound = nullptr);
 
   /* A class representing a list of FrameConstructionItems.  Instances of this
      class are only created as AutoFrameConstructionItemList, or as a member
      of FrameConstructionItem. */
   class FrameConstructionItemList
   {
   public:
     void Reset(nsCSSFrameConstructor* aFCtor)
--- a/layout/reftests/css-display/display-contents-acid-ref.html
+++ b/layout/reftests/css-display/display-contents-acid-ref.html
@@ -169,16 +169,12 @@ 0
 <div class="contents c3"><div>3</div></div>
 </div>
 
 <span class="c2 b"><legend class="inline c1">Legend</legend><legend class="inline c1">Legend</legend></span>
 <br clear="all">
 <span class="c3">x<div class="inline c1">float:left</div></span>
 <span class="c3">y<div class="inline c1">position:absolute</div></span>
 
-<!--  -->
-
-<fieldset class="c1" style="display:inline"><legend>Legend</legend>fieldset</fieldset>
-<button class="c1">but<span>ton</span></button>
-<select class="c1"><option>select</select>
+<span class="c1">Legendfieldset but<span>ton</span></span>
 
 </body>
 </html>
--- a/layout/reftests/css-display/display-contents-acid.html
+++ b/layout/reftests/css-display/display-contents-acid.html
@@ -176,16 +176,17 @@ 0
 <div class="contents c3"><!-- comment node --></div>
 <div class="contents c3"><?PI ?></div>
 
 <span class="c2"><legend class="contents c1">Legend</legend><legend class="contents c1">Legend</legend></span>
 <br clear="all">
 <span class="c3">x<div class="contents c1" style="float:left">float:left</div></span>
 <span class="c3">y<div class="contents c1" style="position:absolute">position:absolute</div></span>
 
-<!-- Stuff below should simply ignore having display:contents -->
+<fieldset class="contents c1"><legend class="contents">Legend</legend>fieldset</fieldset>
+<button class="contents c1" style="font: inherit;">but<span>ton</span></button>
 
-<fieldset class="contents c1"><legend class="contents">Legend</legend>fieldset</fieldset>
-<button class="contents c1">but<span>ton</span></button>
+<!-- Stuff below should simply behave as having display: none -->
+
 <select class="contents c1"><option>select</select>
 
 </body>
 </html>
--- a/layout/reftests/css-display/display-contents-fieldset-ref.html
+++ b/layout/reftests/css-display/display-contents-fieldset-ref.html
@@ -18,35 +18,43 @@ legend { border: 1px solid; }
 </style>
 </head>
 <body>
 <fieldset><div class="test contents"></div></fieldset>
 <fieldset><div class="test contents">x</div></fieldset>
 <fieldset><div class="test contents after"></div></fieldset>
 <fieldset><div class="test contents before"></div></fieldset>
 <fieldset><div class="test contents before after"></div></fieldset>
-<fieldset><legend class="test contents"></legend></fieldset>
-<fieldset><legend class="test contents" style="padding:0"></legend></fieldset>
-<fieldset><legend class="contents"><div class="test contents"></div></legend></fieldset>
+<fieldset><span></span></fieldset>
+<fieldset><span></span></fieldset>
+<fieldset><span></span></fieldset>
 <fieldset class="test2"></fieldset>
 <fieldset class="test2 after"></fieldset>
 <fieldset class="test2"><legend class="static"></legend></fieldset>
 <fieldset class="test2"><legend class="static contents"></legend></fieldset>
 <fieldset class="test2"><legend class="static" style="padding:0"></legend></fieldset>
 <fieldset class="test2 p0"></fieldset>
-<fieldset class="test2 p0"></fieldset>
+<fieldset class="test3 p0"></fieldset>
 <fieldset class="test2 p0"><legend class="static"></legend></fieldset>
-<fieldset class="test2 p0"><legend class="static"></legend></fieldset>
+<fieldset class="test3 p0"><legend class="static"></legend></fieldset>
 <script>
 document.body.offsetHeight;
 var tests = document.querySelectorAll('.test');
 for (i=0; i < tests.length; ++i) {
   test = tests[i];
   test.appendChild(document.createElement('span'));
 }
-var tests = document.querySelectorAll('.test2,.test3');
+var tests = document.querySelectorAll('.test2');
+for (i=0; i < tests.length; ++i) {
+  test = tests[i];
+  let span = document.createElement('dummy-inline');
+  span.innerHTML = "legend";
+  test.appendChild(span);
+}
+
+var tests = document.querySelectorAll('.test3');
 for (i=0; i < tests.length; ++i) {
   test = tests[i];
   test.appendChild(document.createElement('legend'));
 }
 </script>
 </body>
 </html>
--- a/layout/svg/nsSVGUseFrame.cpp
+++ b/layout/svg/nsSVGUseFrame.cpp
@@ -161,18 +161,20 @@ nsSVGUseFrame::NotifySVGChanged(uint32_t
 }
 
 //----------------------------------------------------------------------
 // nsIAnonymousContentCreator methods:
 
 nsresult
 nsSVGUseFrame::CreateAnonymousContent(nsTArray<ContentInfo>& aElements)
 {
-  SVGUseElement *use = static_cast<SVGUseElement*>(GetContent());
-
+  // FIXME(emilio): This should not be done at frame construction time, but
+  // using Shadow DOM or something like that instead, to support properly
+  // display: contents in <svg:use>.
+  auto* use = static_cast<SVGUseElement*>(GetContent());
   mContentClone = use->CreateAnonymousContent();
   nsLayoutUtils::PostRestyleEvent(
     use, nsRestyleHint(0), nsChangeHint_InvalidateRenderingObservers);
   if (!mContentClone)
     return NS_ERROR_FAILURE;
   aElements.AppendElement(mContentClone);
   return NS_OK;
 }