Bug 1453702: [css-display] Move unboxing to style, and handle display: contents before other suppressions. r=mats,xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 14 Apr 2018 18:38:29 +0200
changeset 467369 43862b2ee72eec858f509ce3b9cb5c6fa96194ba
parent 467368 55c90c91e3f3844e6db5b99c86c1ec6a0c24758d
child 467370 438494d2d17bec92e4f4e38661a85b60680ab087
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, xidorn
bugs1453702, 1303605
milestone61.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 1453702: [css-display] Move unboxing to style, and handle display: contents before other suppressions. r=mats,xidorn This also adopts the resolution of [1] while at it, and switches XUL to not support display: contents until a use case appears. This makes our behavior consistent both with the spec and also in terms of handling dynamic changes to stuff that would otherwise get suppressed. Also makes us consistent with both Blink and WebKit in terms of computed style. We were the only ones respecting "behaves as display: none" without actually computing to display: none. Will file a spec issue to get that changed. It also makes us match Blink and WebKit in terms of respecting display: contents before other suppressions, see the reftest which I didn't write as a WPT (because there's no spec supporting neither that or the opposite of what we do), where a <g> element respects display: contents even though if it had any other kind of display value we'd suppress the frame for it and all the descendants since it's an SVG element in a non-SVG subtree. Also, this removes the page-break bit from the display: contents loop, which I think is harmless. As long as the tests under style are based in namespace id / node name / traversal parent, this should not make style sharing go wrong in any way, since that's the first style sharing check we do at [2]. The general idea under this change is making all nodes with computed style of display: contents actually honor it. Otherwise there's no way of making the setup sound except re-introducing something similar to all the state tracking removed in bug 1303605. [1]: https://github.com/w3c/csswg-drafts/issues/2167 [2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700 MozReview-Commit-ID: JoCKnGYEleD
layout/base/crashtests/1453702.html
layout/base/crashtests/crashtests.list
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/reftests/css-display/display-contents-suppression-dynamic-ref.html
layout/reftests/css-display/display-contents-suppression-dynamic.html
layout/reftests/css-display/reftest.list
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/style_adjuster.rs
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/css-display/display-contents-details-001-ref.html
testing/web-platform/tests/css/css-display/display-contents-details-001.html
testing/web-platform/tests/css/css-display/display-contents-suppression-dynamic-001-ref.html
testing/web-platform/tests/css/css-display/display-contents-suppression-dynamic-001.html
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1453702.html
@@ -0,0 +1,16 @@
+<style></style>
+<script id='script'>
+  function start() {
+    try { o1 = document.createElement('p') } catch (e) {}
+    try { o2 = document.createElement('l') } catch (e) {}
+    try { o3 = document.createElement('c') } catch (e) {}
+    try { o4 = document.createElement('textarea') } catch (e) {}
+    try { document.documentElement.appendChild(o4) } catch (e) {}
+    try { o4.scrollLeft = 8 } catch (e) {}
+    try { o4.appendChild(document.getElementById('script')) } catch (e) {}
+    try { document.styleSheets[0].insertRule("* { display:contents !important }", 0) } catch (e) {}
+    try { o3.convertPointFromNode({ }, o1, { }) } catch (e) {}
+    try { document.getElementById('script').appendChild(o2) } catch (e) {}
+  }
+  document.addEventListener('DOMContentLoaded', start)
+</script>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -522,8 +522,9 @@ load 1429961.html
 load 1435015.html
 load 1429962.html
 pref(dom.webcomponents.shadowdom.enabled,true) load 1439016.html
 load 1442506.html
 load 1437155.html
 load 1443027-1.html
 load 1448841-1.html
 load 1452839.html
+load 1453702.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3494,30 +3494,23 @@ nsCSSFrameConstructor::FindDataByInt(int
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindDataByTag(nsAtom* aTag,
                                      Element* aElement,
                                      ComputedStyle* aComputedStyle,
                                      const FrameConstructionDataByTag* aDataPtr,
-                                     uint32_t aDataLength,
-                                     bool* aTagFound)
-{
-  if (aTagFound) {
-    *aTagFound = false;
-  }
+                                     uint32_t aDataLength)
+{
   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, aComputedStyle);
       }
 
       return data;
     }
   }
@@ -3611,39 +3604,18 @@ 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)
   };
 
-  bool tagFound;
-  const FrameConstructionData* data =
-    FindDataByTag(aTag, aElement, aComputedStyle, sHTMLData,
-                  ArrayLength(sHTMLData), &tagFound);
-
-  // https://drafts.csswg.org/css-display/#unbox-html
-  if (tagFound &&
-      MOZ_UNLIKELY(aComputedStyle->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;
+  return FindDataByTag(aTag, aElement, aComputedStyle, sHTMLData,
+                       ArrayLength(sHTMLData));
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindImgData(Element* aElement,
                                    ComputedStyle* aComputedStyle)
 {
   if (!nsImageFrame::ShouldCreateImageFrameFor(aElement, aComputedStyle)) {
@@ -4338,34 +4310,20 @@ nsCSSFrameConstructor::FindXULTagData(El
     SIMPLE_XUL_CREATE(splitter, NS_NewSplitterFrame),
     SIMPLE_TAG_CHAIN(listboxbody,
                      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)
-};
-
-  bool tagFound;
-  const FrameConstructionData* data =
-    FindDataByTag(aTag, aElement, aComputedStyle, 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(aComputedStyle->StyleDisplay()->mDisplay ==
-                   StyleDisplay::Contents)) {
-    static const FrameConstructionData sSuppressData = SUPPRESS_FCDATA();
-    return &sSuppressData;
-  }
-
-  return data;
+  };
+
+  return FindDataByTag(aTag, aElement, aComputedStyle, sXULTagData,
+                       ArrayLength(sXULTagData));
 }
 
 #ifdef MOZ_XUL
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindPopupGroupData(Element* aElement,
                                           ComputedStyle* /* unused */)
 {
@@ -5310,33 +5268,16 @@ 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 (aComputedStyle->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 =
@@ -5606,16 +5547,84 @@ nsCSSFrameConstructor::SetAsUndisplayedC
     if (aIsGeneratedContent) {
       aContent->UnbindFromTree();
     }
     return;
   }
   MOZ_ASSERT(!aIsGeneratedContent, "Should have had pseudo type");
 }
 
+// Whether we should suppress frames for a child under a <select> frame.
+//
+// Never create frames for non-option/optgroup kids of <select> and non-option
+// kids of <optgroup> inside a <select>.
+// XXXbz it's not clear how this should best work with XBL.
+static bool
+ShouldSuppressFrameInSelect(const nsIContent* aParent,
+                            const nsIContent* aChild)
+{
+  if (!aParent ||
+      !aParent->IsAnyOfHTMLElements(nsGkAtoms::select, nsGkAtoms::optgroup)) {
+    return false;
+  }
+
+  // If we're in any display: contents subtree, just suppress the frame.
+  //
+  // We can't be regular NAC, since display: contents has no frame to generate
+  // them off.
+  if (aChild->GetParent() != aParent) {
+    return true;
+  }
+
+  // Option is always fine.
+  if (aChild->IsHTMLElement(nsGkAtoms::option)) {
+    return false;
+  }
+
+  // <optgroup> is OK in <select> but not in <optgroup>.
+  if (aChild->IsHTMLElement(nsGkAtoms::optgroup) &&
+      aParent->IsHTMLElement(nsGkAtoms::select)) {
+    return false;
+  }
+
+  // Allow native anonymous content no matter what.
+  if (aChild->IsRootOfAnonymousSubtree()) {
+    return false;
+  }
+
+  return true;
+}
+
+static bool
+ShouldSuppressFrameInNonOpenDetails(const HTMLDetailsElement* aDetails,
+                                    const nsIContent* aChild)
+{
+  if (!aDetails || aDetails->Open()) {
+    return false;
+  }
+
+  if (aChild->GetParent() != aDetails) {
+    return true;
+  }
+
+  auto* summary = HTMLSummaryElement::FromNode(aChild);
+  if (summary && summary->IsMainSummary()) {
+    return false;
+  }
+
+  // Don't suppress NAC, unless it's ::before or ::after.
+  if (aChild->IsRootOfAnonymousSubtree() &&
+      !aChild->IsGeneratedContentContainerForBefore() &&
+      !aChild->IsGeneratedContentContainerForAfter()) {
+    return false;
+  }
+
+  return true;
+}
+
 void
 nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState& aState,
                                                          nsIContent* aContent,
                                                          nsContainerFrame* aParentFrame,
                                                          bool aSuppressWhiteSpaceOptimizations,
                                                          ComputedStyle* aComputedStyle,
                                                          uint32_t aFlags,
                                                          nsTArray<nsIAnonymousContentCreator::ContentInfo>* aAnonChildren,
@@ -5678,67 +5687,74 @@ nsCSSFrameConstructor::AddFrameConstruct
       }
     }
   }
 
   const bool isGeneratedContent = !!(aFlags & ITEM_IS_GENERATED_CONTENT);
 
   // Pre-check for display "none" - if we find that, don't create
   // any frame at all
-  if (StyleDisplay::None == display->mDisplay) {
+  if (display->mDisplay == StyleDisplay::None) {
     SetAsUndisplayedContent(aState, aItems, aContent, computedStyle,
                             isGeneratedContent);
     return;
   }
 
+  if (display->mDisplay == StyleDisplay::Contents) {
+    if (aParentFrame) {
+      // FIXME(emilio): Pretty sure aParentFrame can't be null here.
+      aParentFrame->AddStateBits(NS_FRAME_MAY_HAVE_GENERATED_CONTENT);
+    }
+    CreateGeneratedContentItem(aState, aParentFrame, aContent->AsElement(),
+                               computedStyle, CSSPseudoElementType::before,
+                               aItems);
+
+    FlattenedChildIterator iter(aContent);
+    for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) {
+      if (!ShouldCreateItemsForChild(aState, child, aParentFrame)) {
+        continue;
+      }
+
+      RefPtr<ComputedStyle> childContext = ResolveComputedStyle(child);
+      DoAddFrameConstructionItems(aState, child, childContext,
+                                  aSuppressWhiteSpaceOptimizations,
+                                  aParentFrame, aAnonChildren, aItems);
+    }
+    aItems.SetParentHasNoXBLChildren(!iter.XBLInvolved());
+
+    CreateGeneratedContentItem(aState, aParentFrame, aContent->AsElement(),
+                               computedStyle, CSSPseudoElementType::after,
+                               aItems);
+    return;
+  }
+
   bool isText = !aContent->IsElement();
 
-  // never create frames for non-option/optgroup kids of <select> and
-  // non-option kids of <optgroup> inside a <select>.
-  // XXXbz it's not clear how this should best work with XBL.
-  nsIContent *parent = aContent->GetParent();
-  if (parent) {
-    // Check tag first, since that check will usually fail
-    if (parent->IsAnyOfHTMLElements(nsGkAtoms::select, nsGkAtoms::optgroup) &&
-        // <option> is ok no matter what
-        !aContent->IsHTMLElement(nsGkAtoms::option) &&
-        // <optgroup> is OK in <select> but not in <optgroup>
-        (!aContent->IsHTMLElement(nsGkAtoms::optgroup) ||
-         !parent->IsHTMLElement(nsGkAtoms::select)) &&
-        // Allow native anonymous content no matter what
-        !aContent->IsRootOfNativeAnonymousSubtree()) {
-      // No frame for aContent
-      if (!isText) {
-        SetAsUndisplayedContent(aState, aItems, aContent, computedStyle,
-                                isGeneratedContent);
-      }
-      return;
-    }
+  nsIContent* parent = aParentFrame ? aParentFrame->GetContent() : nullptr;
+  if (ShouldSuppressFrameInSelect(parent, aContent)) {
+    if (!isText) {
+      SetAsUndisplayedContent(aState, aItems, aContent, computedStyle,
+                              isGeneratedContent);
+    }
+    return;
   }
 
   // When constructing a child of a non-open <details>, create only the frame
   // for the main <summary> element, and skip other elements.  This only applies
   // to things that are not roots of native anonymous subtrees (except for
   // ::before and ::after); we always want to create "internal" anonymous
   // content.
   auto* details = HTMLDetailsElement::FromNodeOrNull(parent);
-  if (details && !details->Open() &&
-      (!aContent->IsRootOfNativeAnonymousSubtree() ||
-       aContent->IsGeneratedContentContainerForBefore() ||
-       aContent->IsGeneratedContentContainerForAfter())) {
-    auto* summary = HTMLSummaryElement::FromNodeOrNull(aContent);
-    if (!summary || !summary->IsMainSummary()) {
-      SetAsUndisplayedContent(aState, aItems, aContent, computedStyle,
-                              isGeneratedContent);
-      return;
-    }
+  if (ShouldSuppressFrameInNonOpenDetails(details, aContent)) {
+    SetAsUndisplayedContent(aState, aItems, aContent, computedStyle,
+                            isGeneratedContent);
+    return;
   }
 
   bool isPopup = false;
-  bool foundMathMLData = false;
   // Try to find frame construction data for this content
   const FrameConstructionData* data;
   if (isText) {
     data = FindTextData(aParentFrame);
     if (!data) {
       // Nothing to do here; suppressed text inside SVG
       return;
     }
@@ -5758,17 +5774,16 @@ nsCSSFrameConstructor::AddFrameConstruct
 
     data = FindHTMLData(element, tag, namespaceId, aParentFrame,
                         computedStyle);
     if (!data) {
       data = FindXULTagData(element, tag, namespaceId, computedStyle);
     }
     if (!data) {
       data = FindMathMLData(element, tag, namespaceId, computedStyle);
-      foundMathMLData = !!data;
     }
     if (!data) {
       data = FindSVGData(element, tag, namespaceId, aParentFrame,
                          aFlags & ITEM_IS_WITHIN_SVG_TEXT,
                          aFlags & ITEM_ALLOWS_TEXT_PATH_CHILD,
                          computedStyle);
     }
 
@@ -5777,17 +5792,17 @@ nsCSSFrameConstructor::AddFrameConstruct
       data = FindXULDisplayData(display, element, computedStyle);
     }
 
     // And general display types
     if (!data) {
       data = FindDisplayData(display, element, computedStyle);
     }
 
-    NS_ASSERTION(data, "Should have frame construction data now");
+    MOZ_ASSERT(data, "Should have frame construction data now");
 
     if (data->mBits & FCDATA_SUPPRESS_FRAME) {
       SetAsUndisplayedContent(aState, aItems, element, computedStyle, isGeneratedContent);
       return;
     }
 
 #ifdef MOZ_XUL
     if ((data->mBits & FCDATA_IS_POPUP) &&
@@ -5820,53 +5835,19 @@ nsCSSFrameConstructor::AddFrameConstruct
     !display->IsAbsolutelyPositionedStyle() &&
     !(aParentFrame && aParentFrame->IsGridContainerFrame()) &&
     !(bits & FCDATA_IS_TABLE_PART) && !(bits & FCDATA_IS_SVG_TEXT);
 
   if (canHavePageBreak && display->mBreakBefore) {
     AddPageBreakItem(aContent, aItems);
   }
 
-  // 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 &&
-      !foundMathMLData) {
-    if (aParentFrame) {
-      aParentFrame->AddStateBits(NS_FRAME_MAY_HAVE_GENERATED_CONTENT);
-    }
-    CreateGeneratedContentItem(aState, aParentFrame, aContent->AsElement(),
-                               computedStyle, CSSPseudoElementType::before,
-                               aItems);
-
-    FlattenedChildIterator iter(aContent);
-    for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) {
-      if (!ShouldCreateItemsForChild(aState, child, aParentFrame)) {
-        continue;
-      }
-
-      RefPtr<ComputedStyle> childContext = ResolveComputedStyle(child);
-      DoAddFrameConstructionItems(aState, child, childContext,
-                                  aSuppressWhiteSpaceOptimizations,
-                                  aParentFrame, aAnonChildren, aItems);
-    }
-    aItems.SetParentHasNoXBLChildren(!iter.XBLInvolved());
-
-    CreateGeneratedContentItem(aState, aParentFrame, aContent->AsElement(),
-                               computedStyle, CSSPseudoElementType::after,
-                               aItems);
-    if (canHavePageBreak && display->mBreakAfter) {
-      AddPageBreakItem(aContent, aItems);
-    }
-    return;
-  }
-
   FrameConstructionItem* item = nullptr;
   if (details && details->Open()) {
-    auto* summary = HTMLSummaryElement::FromNodeOrNull(aContent);
+    auto* summary = HTMLSummaryElement::FromNode(aContent);
     if (summary && summary->IsMainSummary()) {
       // If details is open, the main summary needs to be rendered as if it is
       // the first child, so add the item to the front of the item list.
       item = aItems.PrependItem(this, data, aContent, pendingBinding,
                                 computedStyle.forget(),
                                 aSuppressWhiteSpaceOptimizations,
                                 aAnonChildren);
     }
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -805,18 +805,17 @@ private:
    * 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,
                   ComputedStyle* aComputedStyle,
                   const FrameConstructionDataByTag* aDataPtr,
-                  uint32_t aDataLength,
-                  bool* aTagFound = nullptr);
+                  uint32_t aDataLength);
 
   /* 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)
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-display/display-contents-suppression-dynamic-ref.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<body>
+<script>
+  const SVG_NS = "http://www.w3.org/2000/svg";
+  let g = document.createElementNS(SVG_NS, "g");
+  g.style.display = "contents";
+  document.body.appendChild(g);
+  let div = document.createElement('div');
+  div.style.width = div.style.height = "100px";
+  div.style.backgroundColor = "green";
+  g.appendChild(div);
+</script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-display/display-contents-suppression-dynamic.html
@@ -0,0 +1,16 @@
+<!doctype html>
+<body>
+<script>
+  const SVG_NS = "http://www.w3.org/2000/svg";
+  let g = document.createElementNS(SVG_NS, "g");
+  g.style.display = "contents";
+  document.body.appendChild(g);
+  // The only difference between this test and the ref is this flush, this
+  // tests we're consistent.
+  g.offsetTop;
+  let div = document.createElement('div');
+  div.style.width = div.style.height = "100px";
+  div.style.backgroundColor = "green";
+  g.appendChild(div);
+</script>
+</body>
--- a/layout/reftests/css-display/reftest.list
+++ b/layout/reftests/css-display/reftest.list
@@ -25,8 +25,9 @@ asserts(0-1) fuzzy-if(Android,8,3216) ==
 == display-contents-xbl-6.xhtml display-contents-xbl-6-ref.html
 == display-contents-xbl-7.xhtml display-contents-xbl-7-ref.html
 == display-contents-list-item-child.html display-contents-list-item-child-ref.html
 == display-contents-dyn-insert-text.html display-contents-dyn-insert-text-ref.html
 == display-contents-writing-mode-1.html display-contents-writing-mode-1-ref.html
 == display-contents-writing-mode-2.html display-contents-writing-mode-2-ref.html
 needs-focus == display-contents-state-change.html display-contents-state-change-ref.html
 == display-flow-root-001.html display-flow-root-001-ref.html
+== display-contents-suppression-dynamic.html display-contents-suppression-dynamic-ref.html
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -429,16 +429,25 @@ pub trait TElement:
     where
         F: FnMut(Self),
     {
     }
 
     /// Return whether this element is an element in the HTML namespace.
     fn is_html_element(&self) -> bool;
 
+    /// Return whether this element is an element in the MathML namespace.
+    fn is_mathml_element(&self) -> bool;
+
+    /// Return whether this element is an element in the SVG namespace.
+    fn is_svg_element(&self) -> bool;
+
+    /// Return whether this element is an element in the XUL namespace.
+    fn is_xul_element(&self) -> bool { false }
+
     /// Return the list of slotted nodes of this node.
     fn slotted_nodes(&self) -> &[Self::ConcreteNode] {
         &[]
     }
 
     /// For a given NAC element, return the closest non-NAC ancestor, which is
     /// guaranteed to exist.
     fn closest_non_native_anonymous_ancestor(&self) -> Option<Self> {
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -669,21 +669,16 @@ impl<'le> GeckoElement<'le> {
     }
 
     #[inline]
     fn namespace_id(&self) -> i32 {
         self.as_node().node_info().mInner.mNamespaceID
     }
 
     #[inline]
-    fn is_xul_element(&self) -> bool {
-        self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32)
-    }
-
-    #[inline]
     fn has_id(&self) -> bool {
         self.as_node()
             .get_bool_flag(nsINode_BooleanFlag::ElementHasID)
     }
 
     #[inline]
     fn state_internal(&self) -> u64 {
         if !self.as_node()
@@ -821,17 +816,17 @@ impl<'le> GeckoElement<'le> {
     #[inline]
     fn is_root_of_use_element_shadow_tree(&self) -> bool {
         if !self.is_root_of_anonymous_subtree() {
             return false;
         }
         match self.parent_element() {
             Some(e) => {
                 e.local_name() == &*local_name!("use") &&
-                    e.namespace() == &*ns!("http://www.w3.org/2000/svg")
+                e.is_svg_element()
             },
             None => false,
         }
     }
 
     fn css_transitions_info(&self) -> FnvHashMap<LonghandId, Arc<AnimationValue>> {
         use gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt;
         use gecko_bindings::bindings::Gecko_ElementTransitions_Length;
@@ -1053,17 +1048,32 @@ impl<'le> TElement for GeckoElement<'le>
     }
 
     fn after_pseudo_element(&self) -> Option<Self> {
         self.before_or_after_pseudo(/* is_before = */ false)
     }
 
     #[inline]
     fn is_html_element(&self) -> bool {
-        self.namespace_id() == (structs::root::kNameSpaceID_XHTML as i32)
+        self.namespace_id() == structs::kNameSpaceID_XHTML as i32
+    }
+
+    #[inline]
+    fn is_mathml_element(&self) -> bool {
+        self.namespace_id() == structs::kNameSpaceID_MathML as i32
+    }
+
+    #[inline]
+    fn is_svg_element(&self) -> bool {
+        self.namespace_id() == structs::kNameSpaceID_SVG as i32
+    }
+
+    #[inline]
+    fn is_xul_element(&self) -> bool {
+        self.namespace_id() == structs::root::kNameSpaceID_XUL as i32
     }
 
     /// Return the list of slotted nodes of this node.
     #[inline]
     fn slotted_nodes(&self) -> &[Self::ConcreteNode] {
         if !self.is_html_slot_element() || !self.as_node().is_in_shadow_tree() {
             return &[];
         }
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! A struct to encapsulate all the style fixups and flags propagations
 //! a computed style needs in order for it to adhere to the CSS spec.
 
+use Atom;
 use app_units::Au;
 use dom::TElement;
 use properties::{self, CascadeFlags, ComputedValues, StyleBuilder};
 use properties::computed_value_flags::ComputedValueFlags;
 use properties::longhands::display::computed_value::T as Display;
 use properties::longhands::float::computed_value::T as Float;
 use properties::longhands::overflow_x::computed_value::T as Overflow;
 use properties::longhands::position::computed_value::T as Position;
@@ -18,16 +19,94 @@ use properties::longhands::position::com
 ///
 /// NOTE(emilio): If new adjustments are introduced that depend on reset
 /// properties of the parent, you may need tweaking the
 /// `ChildCascadeRequirement` code in `matching.rs`.
 pub struct StyleAdjuster<'a, 'b: 'a> {
     style: &'a mut StyleBuilder<'b>,
 }
 
+#[cfg(feature = "gecko")]
+fn is_topmost_svg_svg_element<E>(e: E) -> bool
+where
+    E: TElement,
+{
+    debug_assert!(e.is_svg_element());
+    if e.local_name() != &*atom!("svg") {
+        return false;
+    }
+
+    let parent = match e.traversal_parent() {
+        Some(n) => n,
+        None => return true,
+    };
+
+    if !parent.is_svg_element() {
+        return true;
+    }
+
+    parent.local_name() == &*atom!("foreignObject")
+}
+
+// https://drafts.csswg.org/css-display/#unbox
+#[cfg(feature = "gecko")]
+fn is_effective_display_none_for_display_contents<E>(element: E) -> bool
+where
+    E: TElement,
+{
+    // FIXME(emilio): This should be an actual static.
+    lazy_static! {
+        static ref SPECIAL_HTML_ELEMENTS: [Atom; 16] = [
+            atom!("br"), atom!("wbr"), atom!("meter"), atom!("progress"),
+            atom!("canvas"), atom!("embed"), atom!("object"), atom!("audio"),
+            atom!("iframe"), atom!("img"), atom!("video"), atom!("frame"),
+            atom!("frameset"), atom!("input"), atom!("textarea"),
+            atom!("select"),
+        ];
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-svg
+    //
+    // There's a note about "Unknown elements", but there's not a good way to
+    // know what that means, or to get that information from here, and no other
+    // UA implements this either.
+    lazy_static! {
+        static ref SPECIAL_SVG_ELEMENTS: [Atom; 6] = [
+            atom!("svg"), atom!("a"), atom!("g"), atom!("use"),
+            atom!("tspan"), atom!("textPath"),
+        ];
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-html
+    if element.is_html_element() {
+        let local_name = element.local_name();
+        return SPECIAL_HTML_ELEMENTS.iter().any(|name| &**name == local_name);
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-svg
+    if element.is_svg_element() {
+        if is_topmost_svg_svg_element(element) {
+            return true;
+        }
+        let local_name = element.local_name();
+        return !SPECIAL_SVG_ELEMENTS.iter().any(|name| &**name == local_name);
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-mathml
+    //
+    // We always treat XUL as display: none. We don't use display:
+    // contents in XUL anyway, so should be fine to be consistent with
+    // MathML unless there's a use case for it.
+    if element.is_mathml_element() || element.is_xul_element() {
+        return true;
+    }
+
+    false
+}
+
 impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
     /// Trivially constructs a new StyleAdjuster.
     #[inline]
     pub fn new(style: &'a mut StyleBuilder<'b>) -> Self {
         StyleAdjuster { style }
     }
 
     /// <https://fullscreen.spec.whatwg.org/#new-stacking-layer>
@@ -372,30 +451,45 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
 
         if overflow_x != original_overflow_x || overflow_y != original_overflow_y {
             let box_style = self.style.mutate_box();
             box_style.set_overflow_x(overflow_x);
             box_style.set_overflow_y(overflow_y);
         }
     }
 
-    /// Native anonymous content converts display:contents into display:inline.
+    /// Handles the relevant sections in:
+    ///
+    /// https://drafts.csswg.org/css-display/#unbox-html
+    ///
+    /// And forbidding display: contents in pseudo-elements, at least for now.
     #[cfg(feature = "gecko")]
-    fn adjust_for_prohibited_display_contents(&mut self) {
-        // TODO: We should probably convert display:contents into display:none
-        // in some cases too: https://drafts.csswg.org/css-display/#unbox
-        //
-        // FIXME(emilio): ::before and ::after should support display: contents,
-        // see bug 1418138.
-        if self.style.pseudo.is_none() || self.style.get_box().clone_display() != Display::Contents
-        {
+    fn adjust_for_prohibited_display_contents<E>(&mut self, element: Option<E>)
+    where
+        E: TElement,
+    {
+        if self.style.get_box().clone_display() != Display::Contents {
             return;
         }
 
-        self.style.mutate_box().set_display(Display::Inline);
+        // FIXME(emilio): ::before and ::after should support display: contents,
+        // see bug 1418138.
+        if self.style.pseudo.is_some() {
+            self.style.mutate_box().set_display(Display::Inline);
+            return;
+        }
+
+        let element = match element {
+            Some(e) => e,
+            None => return,
+        };
+
+        if is_effective_display_none_for_display_contents(element) {
+            self.style.mutate_box().set_display(Display::None);
+        }
     }
 
     /// If a <fieldset> has grid/flex display type, we need to inherit
     /// this type into its ::-moz-fieldset-content anonymous box.
     ///
     /// NOTE(emilio): We don't need to handle the display change for this case
     /// in matching.rs because anonymous box restyling works separately to the
     /// normal cascading process.
@@ -652,17 +746,17 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         // cascade most properties anyway, and they wouldn't be looked up.
         if flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) {
             return;
         }
 
         self.adjust_for_visited(element);
         #[cfg(feature = "gecko")]
         {
-            self.adjust_for_prohibited_display_contents();
+            self.adjust_for_prohibited_display_contents(element);
             self.adjust_for_fieldset_content(layout_parent_style);
         }
         self.adjust_for_top_layer();
         self.blockify_if_necessary(layout_parent_style, element);
         self.adjust_for_position();
         self.adjust_for_overflow();
         #[cfg(feature = "gecko")]
         {
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -106498,16 +106498,28 @@
       [
        "/css/css-display/display-contents-pass-ref.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-display/display-contents-details-001.html": [
+    [
+     "/css/css-display/display-contents-details-001.html",
+     [
+      [
+       "/css/css-display/display-contents-details-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-display/display-contents-details.html": [
     [
      "/css/css-display/display-contents-details.html",
      [
       [
        "/css/css-display/display-contents-pass-ref.html",
        "=="
       ]
@@ -106966,16 +106978,28 @@
       [
        "/css/css-display/display-contents-state-change-001-ref.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-display/display-contents-suppression-dynamic-001.html": [
+    [
+     "/css/css-display/display-contents-suppression-dynamic-001.html",
+     [
+      [
+       "/css/css-display/display-contents-suppression-dynamic-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-display/display-contents-svg-elements.html": [
     [
      "/css/css-display/display-contents-svg-elements.html",
      [
       [
        "/css/css-display/display-contents-svg-elements-ref.html",
        "=="
       ]
@@ -238674,16 +238698,21 @@
      {}
     ]
    ],
    "css/css-display/display-contents-block-002-ref.html": [
     [
      {}
     ]
    ],
+   "css/css-display/display-contents-details-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/css-display/display-contents-dynamic-generated-content-fieldset-001-ref.html": [
     [
      {}
     ]
    ],
    "css/css-display/display-contents-dynamic-pseudo-insertion-001-ref.html": [
     [
      {}
@@ -238739,16 +238768,21 @@
      {}
     ]
    ],
    "css/css-display/display-contents-state-change-001-ref.html": [
     [
      {}
     ]
    ],
+   "css/css-display/display-contents-suppression-dynamic-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/css-display/display-contents-svg-elements-ref.html": [
     [
      {}
     ]
    ],
    "css/css-display/display-contents-table-001-ref.html": [
     [
      {}
@@ -490259,16 +490293,24 @@
   "css/css-display/display-contents-button.html": [
    "b00b06641c653af5ab91a6156a726ff2d4ea9b00",
    "reftest"
   ],
   "css/css-display/display-contents-computed-style.html": [
    "bf523ee19c5ff481a3f91f8eb124eb83301738c5",
    "testharness"
   ],
+  "css/css-display/display-contents-details-001-ref.html": [
+   "bee1115524772a38c49bb756971477ff303c4ab0",
+   "support"
+  ],
+  "css/css-display/display-contents-details-001.html": [
+   "7eb2da8b522cddb5f66f9957fa113dabfbfa84af",
+   "reftest"
+  ],
   "css/css-display/display-contents-details.html": [
    "d1f62084c5adc3ba7a86306704f804d4cbe0428c",
    "reftest"
   ],
   "css/css-display/display-contents-dynamic-before-after-001.html": [
    "9142d9f1ad5b5e9a5f5705a44c962084f023a3c6",
    "reftest"
   ],
@@ -490471,16 +490513,24 @@
   "css/css-display/display-contents-state-change-001-ref.html": [
    "f7e25855cc7ef1896a9a52005d3c1379bf74746b",
    "support"
   ],
   "css/css-display/display-contents-state-change-001.html": [
    "0a689fbe90be794772c66d59b033d15336e6dfe3",
    "reftest"
   ],
+  "css/css-display/display-contents-suppression-dynamic-001-ref.html": [
+   "ec636e00c73939e04d5bc8be6f9963a32c970c7c",
+   "support"
+  ],
+  "css/css-display/display-contents-suppression-dynamic-001.html": [
+   "fa1406cc302f6763ef6a85913d5f26470797c677",
+   "reftest"
+  ],
   "css/css-display/display-contents-svg-anchor-child.html": [
    "1debf3811cad0a1ef22a06744c9906cf96c2d990",
    "testharness"
   ],
   "css/css-display/display-contents-svg-elements-ref.html": [
    "3e4ddf5f740d8fa688bcbb24201be0a6d4349017",
    "support"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/display-contents-details-001-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<details>
+  <div>
+    <summary>summary</summary>
+    details
+  </div>
+</details>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/display-contents-details-001.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Test: display: contents under a details element doesn't prevent content from getting suppressed</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://drafts.csswg.org/css-display-3/#valdef-display-contents">
+<link rel="match" href="display-contents-details-001-ref.html">
+<details>
+  <div style="display:contents">
+    <summary>summary</summary>
+    details
+  </div>
+</details>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/display-contents-suppression-dynamic-001-ref.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<p>Test passes if you see nothing below.</p>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/display-contents-suppression-dynamic-001.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Test: display: contents unboxing works in presence of dynamic changes to the tree.</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://drafts.csswg.org/css-display-3/#valdef-display-contents">
+<link rel="help" href="https://drafts.csswg.org/css-display-3/#unbox">
+<link rel="match" href="display-contents-suppression-dynamic-001-ref.html">
+<p>Test passes if you see nothing below.</p>
+<textarea style="display: contents">
+  FAIL
+</textarea>
+<script>
+  let textarea = document.querySelector("textarea");
+  textarea.offsetTop;
+  textarea.appendChild(document.createTextNode("FAIL"));
+</script>