Bug 1872907 - stop marking SVG symbol elements as display:none r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D197760

dom/svg/SVGSymbolElement.cpp

@@ -29,6 +29,20 @@ SVGSymbolElement::SVGSymbolElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
     : SVGSymbolElementBase(std::move(aNodeInfo)) {}
 
+Focusable SVGSymbolElement::IsFocusableWithoutStyle(bool aWithMouse) {
+  if (!CouldBeRendered()) {
+    return {};
+  }
+  return SVGSymbolElementBase::IsFocusableWithoutStyle(aWithMouse);
+}
+
+bool SVGSymbolElement::CouldBeRendered() const {
+  // Only <symbol> elements in the root of a <svg:use> shadow tree are
+  // displayed.
+  auto* shadowRoot = ShadowRoot::FromNodeOrNull(GetParentNode());
+  return shadowRoot && shadowRoot->Host()->IsSVGElement(nsGkAtoms::use);
+}
+
 //----------------------------------------------------------------------
 // nsINode methods
 

dom/svg/SVGSymbolElement.h

@@ -26,11 +26,17 @@ class SVGSymbolElement final : public SVGSymbolElementBase {
   ~SVGSymbolElement() = default;
   JSObject* WrapNode(JSContext* cx, JS::Handle<JSObject*> aGivenProto) override;
 
+  Focusable IsFocusableWithoutStyle(bool aWithMouse) override;
+
  public:
   // interfaces:
   NS_DECL_ISUPPORTS_INHERITED
 
+  NS_IMPL_FROMNODE_WITH_TAG(SVGSymbolElement, kNameSpaceID_SVG, symbol)
+
   nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
+
+  bool CouldBeRendered() const;
 };
 
 }  // namespace mozilla::dom

dom/svg/SVGUseElement.cpp

@@ -16,9 +16,10 @@
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/ReferrerInfo.h"
 #include "mozilla/dom/ShadowIncludingTreeIterator.h"
-#include "mozilla/dom/SVGLengthBinding.h"
 #include "mozilla/dom/SVGGraphicsElement.h"
+#include "mozilla/dom/SVGLengthBinding.h"
 #include "mozilla/dom/SVGSVGElement.h"
+#include "mozilla/dom/SVGSymbolElement.h"
 #include "mozilla/dom/SVGUseElementBinding.h"
 #include "nsGkAtoms.h"
 #include "nsContentUtils.h"
@@ -247,11 +248,8 @@ void SVGUseElement::NodeWillBeDestroyed(nsINode* aNode) {
 
 // Returns whether this node could ever be displayed.
 static bool NodeCouldBeRendered(const nsINode& aNode) {
-  if (aNode.IsSVGElement(nsGkAtoms::symbol)) {
-    // Only <symbol> elements in the root of a <svg:use> shadow tree are
-    // displayed.
-    auto* shadowRoot = ShadowRoot::FromNodeOrNull(aNode.GetParentNode());
-    return shadowRoot && shadowRoot->Host()->IsSVGElement(nsGkAtoms::use);
+  if (const auto* symbol = SVGSymbolElement::FromNode(aNode)) {
+    return symbol->CouldBeRendered();
   }
   // TODO: Do we have other cases we can optimize out easily?
   return true;

layout/style/test/test_non_content_accessible_pseudos.html

@@ -15,7 +15,6 @@ const NON_CONTENT_ACCESIBLE_PSEUDOS = [
   "::-moz-search-clear-button",
 
   ":-moz-native-anonymous",
-  ":-moz-use-shadow-tree-root",
   ":-moz-table-border-nonzero",
   ":-moz-browser-frame",
   ":-moz-devtools-highlighted",

layout/svg/SVGContainerFrame.cpp

@@ -162,9 +162,7 @@ void SVGDisplayContainerFrame::InsertFrames(
     for (nsIFrame* kid = firstNewFrame; kid != nextFrame;
          kid = kid->GetNextSibling()) {
       ISVGDisplayableFrame* SVGFrame = do_QueryFrame(kid);
-      if (SVGFrame) {
-        MOZ_ASSERT(!kid->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY),
-                   "Check for this explicitly in the |if|, then");
+      if (SVGFrame && !kid->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
         bool isFirstReflow = kid->HasAnyStateBits(NS_FRAME_FIRST_REFLOW);
         // Remove bits so that ScheduleBoundsUpdate will work:
         kid->RemoveStateBits(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
@@ -190,8 +188,10 @@ void SVGDisplayContainerFrame::RemoveFrame(DestroyContext& aContext,
   // nsContainerFrame::RemoveFrame, so it doesn't call FrameNeedsReflow. We
   // need to schedule a repaint and schedule an update to our overflow rects.
   SchedulePaint();
-  PresContext()->RestyleManager()->PostRestyleEvent(
-      mContent->AsElement(), RestyleHint{0}, nsChangeHint_UpdateOverflow);
+  if (!HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
+    PresContext()->RestyleManager()->PostRestyleEvent(
+        mContent->AsElement(), RestyleHint{0}, nsChangeHint_UpdateOverflow);
+  }
 
   SVGContainerFrame::RemoveFrame(aContext, aListID, aOldFrame);
 }
@@ -332,9 +332,7 @@ void SVGDisplayContainerFrame::ReflowSVG() {
 
   for (auto* kid : mFrames) {
     ISVGDisplayableFrame* SVGFrame = do_QueryFrame(kid);
-    if (SVGFrame) {
-      MOZ_ASSERT(!kid->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY),
-                 "Check for this explicitly in the |if|, then");
+    if (SVGFrame && !kid->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
       SVGFrame->ReflowSVG();
 
       // We build up our child frame overflows here instead of using

layout/svg/SVGSwitchFrame.cpp

@@ -195,9 +195,7 @@ void SVGSwitchFrame::ReflowSVG() {
   ReflowAllSVGTextFramesInsideNonActiveChildren(child);
 
   ISVGDisplayableFrame* svgChild = do_QueryFrame(child);
-  if (svgChild) {
-    MOZ_ASSERT(!child->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY),
-               "Check for this explicitly in the |if|, then");
+  if (svgChild && !child->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
     svgChild->ReflowSVG();
 
     // We build up our child frame overflows here instead of using

layout/svg/SVGSymbolFrame.cpp

@@ -7,6 +7,7 @@
 // Main header first:
 #include "SVGSymbolFrame.h"
 
+#include "mozilla/dom/SVGSymbolElement.h"
 #include "mozilla/PresShell.h"
 
 nsIFrame* NS_NewSVGSymbolFrame(mozilla::PresShell* aPresShell,
@@ -26,14 +27,24 @@ NS_QUERYFRAME_HEAD(SVGSymbolFrame)
   NS_QUERYFRAME_ENTRY(SVGSymbolFrame)
 NS_QUERYFRAME_TAIL_INHERITING(SVGViewportFrame)
 
-#ifdef DEBUG
 void SVGSymbolFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                           nsIFrame* aPrevInFlow) {
-  NS_ASSERTION(aContent->IsSVGElement(nsGkAtoms::symbol),
-               "Content is not an SVG 'symbol' element!");
+  MOZ_ASSERT(aContent->IsSVGElement(nsGkAtoms::symbol),
+             "Content is not an SVG 'symbol' element!");
+
+  if (!dom::SVGSymbolElement::FromNode(aContent)->CouldBeRendered()) {
+    AddStateBits(NS_FRAME_IS_NONDISPLAY);
+  }
 
   SVGViewportFrame::Init(aContent, aParent, aPrevInFlow);
 }
-#endif /* DEBUG */
+
+void SVGSymbolFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
+                                      const nsDisplayListSet& aLists) {
+  if (HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
+    return;
+  }
+  SVGViewportFrame::BuildDisplayList(aBuilder, aLists);
+}
 
 }  // namespace mozilla

layout/svg/SVGSymbolFrame.h

@@ -32,10 +32,11 @@ class SVGSymbolFrame final : public SVGViewportFrame {
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS(SVGSymbolFrame)
 
-#ifdef DEBUG
   void Init(nsIContent* aContent, nsContainerFrame* aParent,
             nsIFrame* aPrevInFlow) override;
-#endif
+
+  void BuildDisplayList(nsDisplayListBuilder* aBuilder,
+                        const nsDisplayListSet& aLists) override;
 
 #ifdef DEBUG_FRAME_DUMP
   nsresult GetFrameName(nsAString& aResult) const override {

layout/svg/svg.css

@@ -11,31 +11,6 @@ style, script {
  display: none;
 }
 
-/*
- * This is only to be overridden by the rule right below.
- *
- * NOTE(emilio): NodeCouldBeRendered in SVGUseElement.cpp relies on this.
- */
-symbol {
- display: none !important;
-}
-
-/*
- * From https://svgwg.org/svg2-draft/struct.html#SymbolNotes:
- *
- * > The generated instance of a 'symbol' that is the direct referenced element
- * > of a 'use' element must always have a computed value of inline for the
- * > display property. In other words, it must be rendered whenever the host
- * > 'use' element is rendered.
- *
- * NOTE(emilio): other browsers instead just replace the `<symbol>` element by
- * an `<svg>` element while cloning, but they don't implement the SVG2
- * selector-matching rules that would make that observable via selectors.
- */
-symbol:-moz-use-shadow-tree-root {
-  display: inline !important;
-}
-
 svg:not(:root), symbol, image, marker, pattern, foreignObject {
  overflow: hidden;
 }

servo/components/style/gecko/non_ts_pseudo_class_list.rs

@@ -87,7 +87,6 @@ macro_rules! apply_non_ts_list {
                 ("-moz-last-node", MozLastNode, _, _),
                 ("-moz-only-whitespace", MozOnlyWhitespace, _, _),
                 ("-moz-native-anonymous", MozNativeAnonymous, _, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS),
-                ("-moz-use-shadow-tree-root", MozUseShadowTreeRoot, _, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS),
                 ("-moz-placeholder", MozPlaceholder, _, _),
 
                 // NOTE(emilio): Pseudo-classes below only depend on document state, and thus

servo/components/style/gecko/wrapper.rs

@@ -801,19 +801,6 @@ impl<'le> GeckoElement<'le> {
         return self.flags() & NODE_IS_NATIVE_ANONYMOUS_ROOT != 0;
     }
 
-    /// Returns true if this node is the shadow root of an use-element shadow tree.
-    #[inline]
-    fn is_root_of_use_element_shadow_tree(&self) -> bool {
-        if !self.as_node().is_in_shadow_tree() {
-            return false;
-        }
-        if !self.parent_node_is_shadow_root() {
-            return false;
-        }
-        let host = self.containing_shadow_host().unwrap();
-        host.is_svg_element() && host.local_name() == &**local_name!("use")
-    }
-
     fn css_transitions_info(&self) -> FxHashMap<OwnedPropertyDeclarationId, Arc<AnimationValue>> {
         use crate::gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt;
         use crate::gecko_bindings::bindings::Gecko_ElementTransitions_Length;
@@ -2072,7 +2059,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
                 true
             },
             NonTSPseudoClass::MozNativeAnonymous => !self.matches_user_and_content_rules(),
-            NonTSPseudoClass::MozUseShadowTreeRoot => self.is_root_of_use_element_shadow_tree(),
             NonTSPseudoClass::MozTableBorderNonzero => unsafe {
                 bindings::Gecko_IsTableBorderNonzero(self.0)
             },

testing/web-platform/tests/svg/struct/reftests/gradient-in-symbol.svg

@@ -0,0 +1,11 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <title>&#x3c;symbol&#x3e; with a gradient inside</title>
+  <h:link rel="help" href="https://www.w3.org/TR/SVG2/struct.html#SymbolElement"/>
+  <h:link rel="match" href="reference/green-100x100.svg"/>
+  <rect fill="url(#a)" width="100" height="100"/>
+  <symbol>
+    <linearGradient id="a">
+      <stop offset="5%" stop-color="green" />
+    </linearGradient>
+  </symbol>
+</svg>

testing/web-platform/tests/svg/struct/reftests/use-symbol-display-none.svg

@@ -0,0 +1,12 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <title>&#x3c;use&#x3e; with a display="none" symbol</title>
+  <h:link rel="help" href="https://svgwg.org/svg2-draft/struct.html#UseElementHrefAttribute"/>
+  <h:link rel="match" href="reference/green-100x100.svg"/>
+  <defs>
+    <symbol id="a" display="none">
+      <rect width="100" height="100" fill="red"/>
+    </symbol>
+  </defs>
+  <rect width="100" height="100" fill="green"/>
+  <use href="#a"></use>
+</svg>