Bug 1421807: Wallpaper SVG text code which makes too many assumptions about the DOM. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 23 Apr 2018 21:23:50 +0200
changeset 471335 26b85ad253292610b73779c14549ba5674b898bc
parent 471334 c4027f932a5276de4d704103e0d0f0361b94d61a
child 471336 f43cf565796e9aa7d2a9c463d8afd69c97941086
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1421807
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 1421807: Wallpaper SVG text code which makes too many assumptions about the DOM. r=heycam Second test-case is because I initially made this code work with display: contents. But then realised that display: contents meant allowing Shadow DOM in there, which I don't really want to deal with right now. MozReview-Commit-ID: HSjFbWEbPAb
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/svg/SVGTextFrame.cpp
layout/svg/crashtests/1421807-1.html
layout/svg/crashtests/1421807-2.html
layout/svg/crashtests/crashtests.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3431,30 +3431,38 @@ FindAncestorWithGeneratedContentPseudo(n
 }
 
 #define SIMPLE_FCDATA(_func) FCDATA_DECL(0, _func)
 #define FULL_CTOR_FCDATA(_flags, _func)                             \
   { _flags | FCDATA_FUNC_IS_FULL_CTOR, { nullptr }, _func, nullptr }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
-nsCSSFrameConstructor::FindTextData(nsIFrame* aParentFrame)
-{
+nsCSSFrameConstructor::FindTextData(nsIFrame* aParentFrame,
+                                    nsIContent* aTextContent)
+{
+  MOZ_ASSERT(aTextContent, "How?");
   if (aParentFrame && IsFrameForSVG(aParentFrame)) {
-    nsIFrame *ancestorFrame =
+    nsIFrame* ancestorFrame =
       nsSVGUtils::GetFirstNonAAncestorFrame(aParentFrame);
-    if (ancestorFrame) {
-      static const FrameConstructionData sSVGTextData =
-        FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT | FCDATA_IS_SVG_TEXT,
-                    NS_NewTextFrame);
-      if (nsSVGUtils::IsInSVGTextSubtree(ancestorFrame)) {
-        return &sSVGTextData;
-      }
-    }
-    return nullptr;
+    if (!ancestorFrame || !nsSVGUtils::IsInSVGTextSubtree(ancestorFrame)) {
+      return nullptr;
+    }
+
+    // Don't render stuff in display: contents / Shadow DOM subtrees, because
+    // TextCorrespondenceRecorder in the SVG text code doesn't really know how
+    // to deal with it. This kinda sucks. :(
+    if (aParentFrame->GetContent() != aTextContent->GetParent()) {
+      return nullptr;
+    }
+
+    static const FrameConstructionData sSVGTextData =
+      FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT | FCDATA_IS_SVG_TEXT,
+                  NS_NewTextFrame);
+    return &sSVGTextData;
   }
 
   static const FrameConstructionData sTextData =
     FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT, NS_NewTextFrame);
   return &sTextData;
 }
 
 void
@@ -5765,17 +5773,17 @@ nsCSSFrameConstructor::AddFrameConstruct
     return;
   }
 
   bool isPopup = false;
   const bool isText = !aContent->IsElement();
   // Try to find frame construction data for this content
   const FrameConstructionData* data;
   if (isText) {
-    data = FindTextData(aParentFrame);
+    data = FindTextData(aParentFrame, aContent);
     if (!data) {
       // Nothing to do here; suppressed text inside SVG
       return;
     }
   } else {
     Element* element = aContent->AsElement();
 
     // Don't create frames for non-SVG element children of SVG elements.
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -1417,17 +1417,18 @@ private:
   nsIFrame* ConstructDetailsFrame(nsFrameConstructorState& aState,
                                   FrameConstructionItem& aItem,
                                   nsContainerFrame* aParentFrame,
                                   const nsStyleDisplay* aStyleDisplay,
                                   nsFrameItems& aFrameItems);
 
   // aParentFrame might be null.  If it is, that means it was an
   // inline frame.
-  static const FrameConstructionData* FindTextData(nsIFrame* aParentFrame);
+  static const FrameConstructionData* FindTextData(nsIFrame* aParentFrame,
+                                                   nsIContent* aTextContent);
 
   void ConstructTextFrame(const FrameConstructionData* aData,
                           nsFrameConstructorState& aState,
                           nsIContent*              aContent,
                           nsContainerFrame*        aParentFrame,
                           ComputedStyle*          aComputedStyle,
                           nsFrameItems&            aFrameItems);
 
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -4577,16 +4577,18 @@ SVGTextFrame::ResolvePositionsForNode(ns
         mPositions[aIndex].mPosition.x = 0.0;
       }
       if (!mPositions[aIndex].IsYSpecified()) {
         mPositions[aIndex].mPosition.y = 0.0;
       }
       mPositions[aIndex].mStartOfChunk = true;
     }
   } else if (!aContent->IsSVGElement(nsGkAtoms::a)) {
+    MOZ_ASSERT(aContent->IsSVGElement());
+
     // We have a text content element that can have x/y/dx/dy/rotate attributes.
     nsSVGElement* element = static_cast<nsSVGElement*>(aContent);
 
     // Get x, y, dx, dy.
     SVGUserUnitList x, y, dx, dy;
     element->GetAnimatedLengthListValues(&x, &y, &dx, &dy, nullptr);
 
     // Get rotate.
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/1421807-1.html
@@ -0,0 +1,5 @@
+<body onload=b.appendChild(a)>
+<div id="a" style="display: contents">
+</div>
+<svg>
+<text id="b">
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/1421807-2.html
@@ -0,0 +1,15 @@
+<style>
+.c1 { display: contents; }
+</style>
+<script>
+function go() {
+  a.attachShadow({mode: "open"}).innerHTML = `<slot> </slot> `;
+  b.appendChild(a);
+}
+</script>
+<body onload=go()>
+<div id="a" class="c1">
+  <span></span>
+</div>
+<svg>
+<text id="b">
--- a/layout/svg/crashtests/crashtests.list
+++ b/layout/svg/crashtests/crashtests.list
@@ -203,8 +203,10 @@ load 1322852.html
 load 1348564.svg
 load 1402109.html
 load 1402124.html
 load 1402486.html
 load conditional-outer-svg-nondirty-reflow-assert.xhtml
 load extref-test-1.xhtml
 load blob-merging-and-retained-display-list.html
 load grouping-empty-bounds.html
+load 1421807-1.html
+pref(dom.webcomponents.shadowdom.enabled,true) load 1421807-2.html