Bug 1578844 - Fix various issues with display: contents within svg text. r=mats
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 12 Oct 2019 16:28:13 +0000
changeset 497363 a0f1057c6f5042b1c80f411a9208a18b46b710cd
parent 497362 09f5cd302da54f83b83cff91a13eea73c7914643
child 497364 43f18d905d75ef27a2e76591e9efba0d9442ffd1
push id97837
push userealvarez@mozilla.com
push dateSat, 12 Oct 2019 16:28:46 +0000
treeherderautoland@a0f1057c6f50 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1578844
milestone71.0a1
Bug 1578844 - Fix various issues with display: contents within svg text. r=mats I'm not happy about all the SVG text / disallow out of flow complexity sprinkled during frame construction... :( Maybe we should add some kind of more generic mechanism to disallow some children for particular kinds of frames, or something. Co-authored-by: Mats Palmgren <mats@mozilla.com> Differential Revision: https://phabricator.services.mozilla.com/D44808
layout/base/crashtests/1578844-1.html
layout/base/crashtests/1578844-2.html
layout/base/crashtests/crashtests.list
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1578844-1.html
@@ -0,0 +1,14 @@
+<script>
+function go() {
+  b.appendChild(a)
+}
+</script>
+<body onload=go()>
+<data>
+<dl id="a" style="display: contents">
+<dl></dl>
+<dd style="position: fixed">
+</dl>
+<svg>
+<text>
+<textPath id="b">
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1578844-2.html
@@ -0,0 +1,20 @@
+<style>
+  dl::before {
+    position: fixed;
+    content: "";
+  }
+</style>
+<script>
+function go() {
+  b.appendChild(a)
+}
+</script>
+<body onload=go()>
+<data>
+<dl id="a" style="display: contents">
+<dl></dl>
+<dd style="position: fixed">
+</dl>
+<svg>
+<text>
+<textPath id="b">
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -576,11 +576,13 @@ pref(layout.css.resizeobserver.enabled,t
 pref(layout.css.column-span.enabled,true) load 1549867.html
 load 1553874.html
 load 1560328.html
 pref(layout.css.column-span.enabled,true) load 1566672.html
 load 1574101-1.html
 load 1574101-2.html
 HTTP load 1575908-1.html
 load 1576972-1.html
+load 1578844-1.html
+load 1578844-2.html
 pref(layout.css.column-span.enabled,true) load 1579953-1.html
 pref(layout.css.column-span.enabled,true) load 1580576.html
 load empty-mask.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -5243,18 +5243,19 @@ bool nsCSSFrameConstructor::ShouldCreate
   }
 
   return true;
 }
 
 void nsCSSFrameConstructor::DoAddFrameConstructionItems(
     nsFrameConstructorState& aState, nsIContent* aContent,
     ComputedStyle* aComputedStyle, bool aSuppressWhiteSpaceOptimizations,
-    nsContainerFrame* aParentFrame, FrameConstructionItemList& aItems) {
-  uint32_t flags = ITEM_ALLOW_XBL_BASE | ITEM_ALLOW_PAGE_BREAK;
+    nsContainerFrame* aParentFrame, FrameConstructionItemList& aItems,
+    uint32_t aFlags) {
+  uint32_t flags = aFlags | ITEM_ALLOW_XBL_BASE | ITEM_ALLOW_PAGE_BREAK;
   if (aParentFrame) {
     if (nsSVGUtils::IsInSVGTextSubtree(aParentFrame)) {
       flags |= ITEM_IS_WITHIN_SVG_TEXT;
     }
     if (aParentFrame->IsBlockFrame() && aParentFrame->GetParent() &&
         aParentFrame->GetParent()->IsSVGTextFrame()) {
       flags |= ITEM_ALLOWS_TEXT_PATH_CHILD;
     }
@@ -5262,25 +5263,25 @@ void nsCSSFrameConstructor::DoAddFrameCo
   AddFrameConstructionItemsInternal(aState, aContent, aParentFrame,
                                     aSuppressWhiteSpaceOptimizations,
                                     aComputedStyle, flags, aItems);
 }
 
 void nsCSSFrameConstructor::AddFrameConstructionItems(
     nsFrameConstructorState& aState, nsIContent* aContent,
     bool aSuppressWhiteSpaceOptimizations, const InsertionPoint& aInsertion,
-    FrameConstructionItemList& aItems) {
+    FrameConstructionItemList& aItems, uint32_t aFlags) {
   nsContainerFrame* parentFrame = aInsertion.mParentFrame;
   if (!ShouldCreateItemsForChild(aState, aContent, parentFrame)) {
     return;
   }
   RefPtr<ComputedStyle> computedStyle = ResolveComputedStyle(aContent);
   DoAddFrameConstructionItems(aState, aContent, computedStyle,
                               aSuppressWhiteSpaceOptimizations, parentFrame,
-                              aItems);
+                              aItems, aFlags);
 }
 
 // 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,
@@ -5488,16 +5489,17 @@ void nsCSSFrameConstructor::AddFrameCons
 
     if (xblInfo.mPendingBinding && xblInfo.mPendingBinding->mBinding) {
       pendingBinding = xblInfo.mPendingBinding.get();
       aState.AddPendingBinding(std::move(xblInfo.mPendingBinding));
     }
   }
 #endif
 
+  const bool withinSVGText = !!(aFlags & ITEM_IS_WITHIN_SVG_TEXT);
   const bool isGeneratedContent = !!(aFlags & ITEM_IS_GENERATED_CONTENT);
   MOZ_ASSERT(!isGeneratedContent || aComputedStyle->IsPseudoElement(),
              "Generated content should be a pseudo-element");
 
   FrameConstructionItem* item = nullptr;
   auto cleanupGeneratedContent = mozilla::MakeScopeExit([&]() {
     if (isGeneratedContent && !item) {
       MOZ_ASSERT(!IsDisplayContents(aContent),
@@ -5513,31 +5515,36 @@ void nsCSSFrameConstructor::AddFrameCons
     return;
   }
 
   if (display.mDisplay == StyleDisplay::Contents) {
     // See the mDisplay fixup code in StyleAdjuster::adjust.
     MOZ_ASSERT(!aContent->AsElement()->IsRootOfNativeAnonymousSubtree(),
                "display:contents on anonymous content is unsupported");
 
-    CreateGeneratedContentItem(aState, aParentFrame, *aContent->AsElement(),
-                               *aComputedStyle, PseudoStyleType::before,
-                               aItems);
+    if (!withinSVGText) {
+      CreateGeneratedContentItem(aState, aParentFrame, *aContent->AsElement(),
+                                 *aComputedStyle, PseudoStyleType::before,
+                                 aItems);
+    }
 
     FlattenedChildIterator iter(aContent);
     InsertionPoint insertion(aParentFrame, aContent);
     for (nsIContent* child = iter.GetNextChild(); child;
          child = iter.GetNextChild()) {
       AddFrameConstructionItems(aState, child, aSuppressWhiteSpaceOptimizations,
-                                insertion, aItems);
+                                insertion, aItems, aFlags);
     }
     aItems.SetParentHasNoXBLChildren(!iter.XBLInvolved());
 
-    CreateGeneratedContentItem(aState, aParentFrame, *aContent->AsElement(),
-                               *aComputedStyle, PseudoStyleType::after, aItems);
+    if (!withinSVGText) {
+      CreateGeneratedContentItem(aState, aParentFrame, *aContent->AsElement(),
+                                 *aComputedStyle, PseudoStyleType::after,
+                                 aItems);
+    }
     return;
   }
 
   nsIContent* parent = aParentFrame ? aParentFrame->GetContent() : nullptr;
   if (ShouldSuppressFrameInSelect(parent, *aContent)) {
     return;
   }
 
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -384,33 +384,35 @@ class nsCSSFrameConstructor final : publ
   // to the list.  This might add more than one item in some rare cases.
   // If aSuppressWhiteSpaceOptimizations is true, optimizations that
   // may suppress the construction of white-space-only text frames
   // must be skipped for these items and items around them.
   void AddFrameConstructionItems(nsFrameConstructorState& aState,
                                  nsIContent* aContent,
                                  bool aSuppressWhiteSpaceOptimizations,
                                  const InsertionPoint& aInsertion,
-                                 FrameConstructionItemList& aItems);
+                                 FrameConstructionItemList& aItems,
+                                 uint32_t aFlags = 0);
 
   // Helper method for AddFrameConstructionItems etc.
   // Unsets the need-frame/restyle bits on aContent.
   // return true iff we should attempt to create frames for aContent.
   bool ShouldCreateItemsForChild(nsFrameConstructorState& aState,
                                  nsIContent* aContent,
                                  nsContainerFrame* aParentFrame);
 
   // Helper method for AddFrameConstructionItems etc.
   // Make sure ShouldCreateItemsForChild() returned true before calling this.
   void DoAddFrameConstructionItems(nsFrameConstructorState& aState,
                                    nsIContent* aContent,
                                    ComputedStyle* aComputedStyle,
                                    bool aSuppressWhiteSpaceOptimizations,
                                    nsContainerFrame* aParentFrame,
-                                   FrameConstructionItemList& aItems);
+                                   FrameConstructionItemList& aItems,
+                                   uint32_t aFlags = 0);
 
   // Construct the frames for the document element.  This can return null if the
   // document element is display:none, or if the document element has a
   // not-yet-loaded XBL binding, or if it's an SVG element that's not <svg>.
   nsIFrame* ConstructDocElementFrame(Element* aDocElement,
                                      nsILayoutHistoryState* aFrameState);
 
   // Set up our mDocElementContainingBlock correctly for the given root