Bug 1427292: [css-display] Update display: contents on Unusual Elements to the spec. r=mats

This will pass[1] whenever the next WPT sync happens. See: https://drafts.csswg.org/css-display/#unbox-html [1]: https://github.com/w3c/web-platform-tests/blob/master/css/css-display/display-contents-unusual-html-elements-none.html MozReview-Commit-ID: 19dqDSxVm7A
2017-12-28 19:48:14 +01:00 · 2017-12-28 19:48:14 +01:00 · 91be560d9c
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@ -3629,13 +3629,20 @@ nsCSSFrameConstructor::FindDataByTag(nsAtom* aTag,
                                     Element* aElement,
                                     nsStyleContext* aStyleContext,
                                     const FrameConstructionDataByTag* aDataPtr,
-                                     uint32_t aDataLength)
+                                     uint32_t aDataLength,
+                                     bool* aTagFound)
 {
+  if (aTagFound) {
+    *aTagFound = false;
+  }
  for (const FrameConstructionDataByTag *curData = aDataPtr,
         *endData = aDataPtr + aDataLength;
       curData != endData;
       ++curData) {
    if (*curData->mTag == aTag) {
+      if (aTagFound) {
+        *aTagFound = true;
+      }
      const FrameConstructionData* data = &curData->mData;
      if (data->mBits & FCDATA_FUNC_IS_DATA_GETTER) {
        return data->mFunc.mDataGetter(aElement, aStyleContext);
@ -3739,8 +3746,29 @@ nsCSSFrameConstructor::FindHTMLData(Element* aElement,
    COMPLEX_TAG_CREATE(details, &nsCSSFrameConstructor::ConstructDetailsFrame)
  };

-  return FindDataByTag(aTag, aElement, aStyleContext, sHTMLData,
-                       ArrayLength(sHTMLData));
+  bool tagFound;
+  const FrameConstructionData* data =
+    FindDataByTag(aTag, aElement, aStyleContext, sHTMLData,
+                  ArrayLength(sHTMLData), &tagFound);
+
+  // https://drafts.csswg.org/css-display/#unbox-html
+  if (tagFound &&
+      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 */
@ -4503,8 +4531,22 @@ nsCSSFrameConstructor::FindXULTagData(Element* aElement,
    SIMPLE_XUL_CREATE(scrollbarbutton, NS_NewScrollbarButtonFrame)
 };

-  return FindDataByTag(aTag, aElement, aStyleContext, sXULTagData,
-                       ArrayLength(sXULTagData));
+  bool tagFound;
+  const FrameConstructionData* data =
+    FindDataByTag(aTag, aElement, aStyleContext, sXULTagData,
+                  ArrayLength(sXULTagData), &tagFound);
+
+  // There's no spec that says what display: contents means for special XUL
+  // elements, but we do the same as for HTML "Unusual Elements", i.e. treat it
+  // as display:none.
+  if (tagFound &&
+      MOZ_UNLIKELY(aStyleContext->StyleDisplay()->mDisplay ==
+                   StyleDisplay::Contents)) {
+    static const FrameConstructionData sSuppressData = SUPPRESS_FCDATA();
+    return &sSuppressData;
+  }
+
+  return data;
 }

 #ifdef MOZ_XUL
@ -4977,8 +5019,7 @@ nsCSSFrameConstructor::FindDisplayData(const nsStyleDisplay* aDisplay,
    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,
@ -5564,6 +5605,23 @@ nsCSSFrameConstructor::FindSVGData(Element* aElement,
    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
@ -6082,7 +6140,11 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
    AddPageBreakItem(aContent, aItems);
  }

-  if (MOZ_UNLIKELY(bits & FCDATA_IS_CONTENTS)) {
+  // FIXME(emilio, https://github.com/w3c/csswg-drafts/issues/2167):
+  //
+  // Figure out what should happen for display: contents in MathML.
+  if (display->mDisplay == StyleDisplay::Contents &&
+      !aContent->IsMathMLElement()) {
    if (!GetDisplayContentsStyleFor(aContent)) {
      MOZ_ASSERT(styleContext->GetPseudo() || !isGeneratedContent,
                 "Should have had pseudo type");
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@ -737,10 +737,6 @@ private:
  /* 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
@ -831,16 +827,21 @@ private:
                  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, aTagFound 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* aTagFound = nullptr);

  /* A class representing a list of FrameConstructionItems.  Instances of this
     class are only created as AutoFrameConstructionItemList, or as a member
--- a/layout/reftests/css-display/display-contents-acid-ref.html
+++ b/layout/reftests/css-display/display-contents-acid-ref.html
@ -174,11 +174,7 @@
 <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
@ -181,10 +181,11 @@
 <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">but<span>ton</span></button>
+<button class="contents c1" style="font: inherit;">but<span>ton</span></button>
+
+<!-- Stuff below should simply behave as having display: none -->
+
 <select class="contents c1"><option>select</select>

 </body>
--- a/layout/reftests/css-display/display-contents-fieldset-ref.html
+++ b/layout/reftests/css-display/display-contents-fieldset-ref.html
@ -23,18 +23,18 @@ legend { border: 1px solid; }
 <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="test2 p0"><legend class="static"></legend></fieldset>
+<fieldset class="test3 p0"></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');
@ -42,7 +42,15 @@ 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'));
--- a/layout/svg/nsSVGUseFrame.cpp
+++ b/layout/svg/nsSVGUseFrame.cpp
@ -166,8 +166,10 @@ nsSVGUseFrame::NotifySVGChanged(uint32_t aFlags)
 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);
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@ -102537,6 +102537,18 @@
     {}
    ]
   ],
+   "css/css-display/display-contents-fieldset-nested-legend.html": [
+    [
+     "/css/css-display/display-contents-fieldset-nested-legend.html",
+     [
+      [
+       "/css/css-display/display-contents-fieldset-nested-legend-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
   "css/css-display/display-contents-first-letter-001.html": [
    [
     "/css/css-display/display-contents-first-letter-001.html",
@ -229858,6 +229870,11 @@
     {}
    ]
   ],
+   "css/css-display/display-contents-fieldset-nested-legend-ref.html": [
+    [
+     {}
+    ]
+   ],
   "css/css-display/display-contents-flex-001-ref.html": [
    [
     {}
@ -469153,6 +469170,14 @@
   "c874f59947d37659e640d4e0abb8a0dc0f03927d",
   "reftest"
  ],
+  "css/css-display/display-contents-fieldset-nested-legend-ref.html": [
+   "89fbd146ffbd862ba3d0f058d8d8c9d8024da0fa",
+   "support"
+  ],
+  "css/css-display/display-contents-fieldset-nested-legend.html": [
+   "45be585ab1523c180b3ba159d25d0dadd80456ac",
+   "reftest"
+  ],
  "css/css-display/display-contents-first-letter-001.html": [
   "acfaf8f68595c7eb36585309b9d6b0bc5c0ffe41",
   "reftest"
@ -504570,11 +504595,11 @@
   "reftest"
  ],
  "css/css-ui/text-overflow-026-ref.html": [
-   "1a7d6d524a217d8fc2252e6434ac1b8fb3d5af2f",
+   "03c0337af58ba7bada64ab36721d9f1fe2f9a2f3",
   "support"
  ],
  "css/css-ui/text-overflow-026.html": [
-   "34cb2b690321f6f93763de2841ebdf5a9275f515",
+   "5087fe90cb3ef8e340be316f1534cb6dba3e0d17",
   "reftest"
  ],
  "css/css-ui/text-overflow-ref.html": [
@ -524414,7 +524439,7 @@
   "support"
  ],
  "dom/events/EventListener-invoke-legacy.html": [
-   "e56b332acb454ab76964b78588536777946ddff8",
+   "13cb3802cb2722417b876ded17fddd222c69564a",
   "testharness"
  ],
  "dom/events/EventListenerOptions-capture.html": [
@ -545950,7 +545975,7 @@
   "testharness"
  ],
  "html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html": [
-   "996d1aa45c5975e13ac0f1e9c9249b3d452ed2e2",
+   "3365d69d34e442962370a1f0ed22017876efe4fe",
   "testharness"
  ],
  "html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html": [
@ -546078,7 +546103,7 @@
   "testharness"
  ],
  "html/semantics/scripting-1/the-script-element/module/load-error-events-inline.html": [
-   "9b35d8d7ba6bb4826274c9570056cc962660d715",
+   "bbdfd11cfc7e52748f07d09ebe7cf6782ad5e782",
   "testharness"
  ],
  "html/semantics/scripting-1/the-script-element/module/load-error-events.html": [
@ -546094,7 +546119,7 @@
   "support"
  ],
  "html/semantics/scripting-1/the-script-element/module/module-in-xhtml.xhtml": [
-   "32a4a4db2a5325b909078c73f76ab51085e66c5a",
+   "4b41e3e07ff72b0f181b0dd3a6c1aab40a01da0c",
   "testharness"
  ],
  "html/semantics/scripting-1/the-script-element/module/module-vs-script-1.html": [
@ -569442,7 +569467,7 @@
   "testharness"
  ],
  "service-workers/service-worker/unregister-then-register-new-script.https.html": [
-   "43964791edbb2b5cd3b3ed27b600e78412902240",
+   "ee8d7cd4e47cdb9bcf2b39c911593bc671df914d",
   "testharness"
  ],
  "service-workers/service-worker/unregister-then-register.https.html": [
--- a/testing/web-platform/tests/css/css-display/display-contents-fieldset-nested-legend-ref.html
+++ b/testing/web-platform/tests/css/css-display/display-contents-fieldset-nested-legend-ref.html
@ -0,0 +1,6 @@
+<!doctype html>
+<meta charset="utf-8">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<title>CSS Test Reference</title>
+<fieldset style="color: green">P<legend style="padding: 0">legend</legend>ASS</fieldset>
--- a/testing/web-platform/tests/css/css-display/display-contents-fieldset-nested-legend.html
+++ b/testing/web-platform/tests/css/css-display/display-contents-fieldset-nested-legend.html
@ -0,0 +1,24 @@
+<!doctype html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<meta charset="utf-8">
+<link rel="match" href="display-contents-fieldset-nested-legend-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css-display/#unbox">
+<link rel="help" href="https://drafts.csswg.org/css-display/#valdef-display-contents">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1427292">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<title>CSS Test: display: contents on legend</title>
+<style>
+fieldset {
+  color: red;
+}
+.contents {
+  display: contents;
+  color: green;
+  border: 10px solid red;
+}
+</style>
+<fieldset><legend class="contents">P<legend>legend</legend>ASS</legend></fieldset>