Bug 1402442: Properly remove display: contents pseudo-frames. r?mats draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 25 Sep 2017 18:25:29 +0200
changeset 675764 008bb7656fca7ec48c2eb460ae791288a1a23312
parent 675748 61d3850239e7f31299fa867526e6d420da63ed66
child 734704 7817e05143a6848656ebf54f8ec5dbb024720a25
push id83235
push userbmo:emilio@crisal.io
push dateThu, 05 Oct 2017 21:03:46 +0000
reviewersmats
bugs1402442
milestone58.0a1
Bug 1402442: Properly remove display: contents pseudo-frames. r?mats MozReview-Commit-ID: 4pjVLQfv3YR Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
layout/base/nsCSSFrameConstructor.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html.ini
testing/web-platform/tests/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001-ref.html
testing/web-platform/tests/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1693,16 +1693,27 @@ nsCSSFrameConstructor::NotifyDestroyingF
     CountersDirty();
   }
 
   RestyleManager()->NotifyDestroyingFrame(aFrame);
 
   nsFrameManager::NotifyDestroyingFrame(aFrame);
 }
 
+static bool
+HasGeneratedContent(const nsIContent* aChild)
+{
+  if (!aChild->MayHaveAnonymousChildren()) {
+    return false;
+  }
+
+  return nsLayoutUtils::GetBeforeFrame(aChild) ||
+         nsLayoutUtils::GetAfterFrame(aChild);
+}
+
 struct nsGenConInitializer {
   nsAutoPtr<nsGenConNode> mNode;
   nsGenConList*           mList;
   void (nsCSSFrameConstructor::*mDirtyAll)();
 
   nsGenConInitializer(nsGenConNode* aNode, nsGenConList* aList,
                       void (nsCSSFrameConstructor::*aDirtyAll)())
     : mNode(aNode), mList(aList), mDirtyAll(aDirtyAll) {}
@@ -8677,25 +8688,24 @@ nsCSSFrameConstructor::ContentRemoved(ns
   if (!childFrame || childFrame->GetContent() != aChild) {
     // XXXbz the GetContent() != aChild check is needed due to bug 135040.
     // Remove it once that's fixed.
     UnregisterDisplayNoneStyleFor(aChild, aContainer);
   }
   MOZ_ASSERT(!childFrame || !GetDisplayContentsStyleFor(aChild),
              "display:contents nodes shouldn't have a frame");
   if (!childFrame && GetDisplayContentsStyleFor(aChild)) {
-    nsIContent* ancestor = aChild->GetFlattenedTreeParent();
-    MOZ_ASSERT(ancestor, "display: contents on the root?");
-    while (!ancestor->GetPrimaryFrame()) {
-      ancestor = ancestor->GetFlattenedTreeParent();
-      MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!");
-    }
-
-    nsIFrame* ancestorFrame = ancestor->GetPrimaryFrame();
-    if (ancestorFrame->GetProperty(nsIFrame::GenConProperty())) {
+    if (HasGeneratedContent(aChild)) {
+      nsIContent* ancestor = aChild->GetFlattenedTreeParent();
+      MOZ_ASSERT(ancestor, "display: contents on the root?");
+      while (!ancestor->GetPrimaryFrame()) {
+        ancestor = ancestor->GetFlattenedTreeParent();
+        MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!");
+      }
+
       // XXXmats Can we recreate frames only for the ::after/::before content?
       // XXX Perhaps even only those that belong to the aChild sub-tree?
       LAYOUT_PHASE_TEMP_EXIT();
       RecreateFramesForContent(ancestor, InsertionKind::Async);
       LAYOUT_PHASE_TEMP_REENTER();
       return true;
     }
 
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -104104,16 +104104,28 @@
       [
        "/css/css-display-3/display-contents-flex-002-ref.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html": [
+    [
+     "/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html",
+     [
+      [
+       "/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-display-3/display-contents-dynamic-inline-flex-001-inline.html": [
     [
      "/css/css-display-3/display-contents-dynamic-inline-flex-001-inline.html",
      [
       [
        "/css/css-display-3/display-contents-inline-flex-001-ref.html",
        "=="
       ]
@@ -229218,16 +229230,21 @@
      {}
     ]
    ],
    "css/css-display-3/display-contents-block-002-ref.html": [
     [
      {}
     ]
    ],
+   "css/css-display-3/display-contents-dynamic-generated-content-fieldset-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/css-display-3/display-contents-flex-001-ref.html": [
     [
      {}
     ]
    ],
    "css/css-display-3/display-contents-flex-002-ref.html": [
     [
      {}
@@ -505751,16 +505768,24 @@
   "css/css-display-3/display-contents-dynamic-flex-003-inline.html": [
    "82c3af46cd3b5e7388df0d2b5f169ac3a454efe6",
    "reftest"
   ],
   "css/css-display-3/display-contents-dynamic-flex-003-none.html": [
    "a2d7c9368ed8c01ca06c36646666270e85aee070",
    "reftest"
   ],
+  "css/css-display-3/display-contents-dynamic-generated-content-fieldset-001-ref.html": [
+   "30ec5c8ddacfbfef8434c37ca7a0a766f2bbc89a",
+   "support"
+  ],
+  "css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html": [
+   "0be8853e66cd7c447725d681a1703a2ad85d8924",
+   "reftest"
+  ],
   "css/css-display-3/display-contents-dynamic-inline-flex-001-inline.html": [
    "40fb07e8ada1530e6835ff2d4e49c5571ffb0baa",
    "reftest"
   ],
   "css/css-display-3/display-contents-dynamic-inline-flex-001-none.html": [
    "40fb07e8ada1530e6835ff2d4e49c5571ffb0baa",
    "reftest"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html.ini
@@ -0,0 +1,4 @@
+[display-contents-dynamic-generated-content-fieldset-001.html]
+  type: reftest
+  expected:
+    if not stylo: FAIL
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001-ref.html
@@ -0,0 +1,16 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Reftest Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<style>
+div {
+  display: contents;
+  border: 10px solid red;
+}
+</style>
+<p>
+  Test passes if there is no red text and no red border.
+</p>
+<fieldset>
+  <div></div>
+</fieldset>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display-3/display-contents-dynamic-generated-content-fieldset-001.html
@@ -0,0 +1,26 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: Dynamic changes to display: contents generated content in fieldsets.</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-dynamic-generated-content-fieldset-001-ref.html">
+<style>
+.after::after {
+  content: "FAIL";
+  color: red;
+}
+div {
+  display: contents;
+  border: 10px solid red;
+}
+</style>
+<p>
+  Test passes if there is no red text and no red border.
+</p>
+<fieldset>
+  <div class="after"></div>
+</fieldset>
+<script>
+document.body.offsetHeight;
+document.querySelector("div").classList.remove("after");
+</script>