Issue #1757 - Reinstate "dom.details_element.enabled" preference

The removal of this preference was botched, all other surrounding plumbing changes appear to be working okay. The ability to use prefs to control this stylesheet might be useful in the future, so perhaps this is one of those "if it ain't broke, don't fix it" bugs where leaving well enough alone in the first place would have been the best choice.

dom/html/HTMLDetailsElement.cpp

@@ -6,11 +6,39 @@
 #include "mozilla/dom/HTMLDetailsElement.h"
 
 #include "mozilla/dom/HTMLDetailsElementBinding.h"
-NS_IMPL_NS_NEW_HTML_ELEMENT(Details)
+#include "mozilla/dom/HTMLUnknownElement.h"
+#include "mozilla/Preferences.h"
+
+// Expand NS_IMPL_NS_NEW_HTML_ELEMENT(Details) to add pref check.
+nsGenericHTMLElement*
+NS_NewHTMLDetailsElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
+                         mozilla::dom::FromParser aFromParser)
+{
+  if (!mozilla::dom::HTMLDetailsElement::IsDetailsEnabled()) {
+    return new mozilla::dom::HTMLUnknownElement(aNodeInfo);
+  }
+
+  return new mozilla::dom::HTMLDetailsElement(aNodeInfo);
+}
 
 namespace mozilla {
 namespace dom {
 
+/* static */ bool
+HTMLDetailsElement::IsDetailsEnabled()
+{
+  static bool isDetailsEnabled = false;
+  static bool added = false;
+
+  if (!added) {
+    Preferences::AddBoolVarCache(&isDetailsEnabled,
+                                 "dom.details_element.enabled");
+    added = true;
+  }
+
+  return isDetailsEnabled;
+}
+
 HTMLDetailsElement::~HTMLDetailsElement()
 {
 }

dom/html/HTMLDetailsElement.h

@@ -23,6 +23,8 @@ class HTMLDetailsElement final : public nsGenericHTMLElement
 public:
   using NodeInfo = mozilla::dom::NodeInfo;
 
+  static bool IsDetailsEnabled();
+
   explicit HTMLDetailsElement(already_AddRefed<NodeInfo>& aNodeInfo)
     : nsGenericHTMLElement(aNodeInfo)
   {

dom/html/HTMLSummaryElement.cpp

@@ -14,7 +14,17 @@
 #include "mozilla/TextEvents.h"
 #include "nsFocusManager.h"
 
-NS_IMPL_NS_NEW_HTML_ELEMENT(Summary)
+// Expand NS_IMPL_NS_NEW_HTML_ELEMENT(Summary) to add pref check.
+nsGenericHTMLElement*
+NS_NewHTMLSummaryElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
+                         mozilla::dom::FromParser aFromParser)
+{
+  if (!mozilla::dom::HTMLDetailsElement::IsDetailsEnabled()) {
+    return new mozilla::dom::HTMLUnknownElement(aNodeInfo);
+  }
+
+  return new mozilla::dom::HTMLSummaryElement(aNodeInfo);
+}
 
 namespace mozilla {
 namespace dom {

dom/webidl/EventHandler.webidl

@@ -95,6 +95,7 @@ interface GlobalEventHandlers {
            [Pref="dom.select_events.enabled"]
            attribute EventHandler onselectstart;
 
+           [Pref="dom.details_element.enabled"]
            attribute EventHandler ontoggle;
 
            // Pointer events handlers

dom/webidl/HTMLDetailsElement.webidl

@@ -11,7 +11,7 @@
  * and create derivative works of this document.
  */
 
-[HTMLConstructor]
+[HTMLConstructor, Pref="dom.details_element.enabled"]
 interface HTMLDetailsElement : HTMLElement {
   [CEReactions, SetterThrows]
   attribute boolean open;

layout/base/nsCSSFrameConstructor.cpp

@@ -3580,6 +3580,10 @@ nsCSSFrameConstructor::FindHTMLData(Element* aElement,
     return nullptr;
   }
 
+  if (aTag == nsGkAtoms::details && !HTMLDetailsElement::IsDetailsEnabled()) {
+    return nullptr;
+  }
+
   static const FrameConstructionDataByTag sHTMLData[] = {
     SIMPLE_TAG_CHAIN(img, nsCSSFrameConstructor::FindImgData),
     SIMPLE_TAG_CHAIN(mozgeneratedcontentimage,
@@ -5791,7 +5795,7 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
   // ::before and ::after); we always want to create "internal" anonymous
   // content.
   auto* details = HTMLDetailsElement::FromContentOrNull(parent);
-  if (details && !details->Open() &&
+  if (details && details->IsDetailsEnabled() && !details->Open() &&
       (!aContent->IsRootOfNativeAnonymousSubtree() ||
        aContent->IsGeneratedContentContainerForBefore() ||
        aContent->IsGeneratedContentContainerForAfter())) {
@@ -5959,7 +5963,7 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
   }
 
   FrameConstructionItem* item = nullptr;
-  if (details && details->Open()) {
+  if (details && details->IsDetailsEnabled() && details->Open()) {
     auto* summary = HTMLSummaryElement::FromContentOrNull(aContent);
     if (summary && summary->IsMainSummary()) {
       // If details is open, the main summary needs to be rendered as if it is

layout/generic/crashtests/crashtests.list

@@ -606,19 +606,19 @@ pref(layout.css.grid.enabled,true) load 1225376.html
 pref(layout.css.grid.enabled,true) load 1225592.html
 load 1229437-1.html
 load 1229437-2.html
-load details-containing-only-text.html
+pref(dom.details_element.enabled,true) load details-containing-only-text.html
-load details-display-none-summary-1.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-1.html
-load details-display-none-summary-2.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-2.html
-load details-display-none-summary-3.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-3.html
-load details-open-overflow-auto.html
+pref(dom.details_element.enabled,true) load details-open-overflow-auto.html
-load details-open-overflow-hidden.html
+pref(dom.details_element.enabled,true) load details-open-overflow-hidden.html
-load details-three-columns.html
+pref(dom.details_element.enabled,true) load details-three-columns.html
 load first-letter-638937-1.html
 load first-letter-638937-2.html
 load flex-nested-abspos-1.html
 pref(dom.meta-viewport.enabled,true) test-pref(font.size.inflation.emPerLine,15) asserts(0-100) load font-inflation-762332.html # bug 762332
 load outline-on-frameset.xhtml
-load summary-position-out-of-flow.html
+pref(dom.details_element.enabled,true) load summary-position-out-of-flow.html
 load text-overflow-bug666751-1.html
 load text-overflow-bug666751-2.html
 load text-overflow-bug670564.xhtml

layout/generic/nsContainerFrame.cpp

@@ -1891,11 +1891,13 @@ nsContainerFrame::RenumberFrameAndDescendants(int32_t* aOrdinal,
   }
 
   // Do not renumber list for summary elements.
+  if (HTMLDetailsElement::IsDetailsEnabled()) {
     HTMLSummaryElement* summary =
       HTMLSummaryElement::FromContent(kid->GetContent());
     if (summary && summary->IsMainSummary()) {
       return false;
     }
+  }
 
   bool kidRenumberedABullet = false;
 

layout/reftests/details-summary/disabled-no-summary-ref.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+  <body>
+    <div>
+      <p>This is the details.</p>
+    </div>
+  </body>
+</html>

layout/reftests/details-summary/disabled-single-summary-ref.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+  <body>
+    <div>
+      <div>Summary</div>
+      <p>This is the details.</p>
+    </div>
+  </body>
+</html>

layout/reftests/details-summary/reftest.list

@@ -1,3 +1,10 @@
+default-preferences pref(dom.details_element.enabled,true)
+
+# Disable <details> and <summary>
+pref(dom.details_element.enabled,false) == single-summary.html disabled-single-summary-ref.html
+pref(dom.details_element.enabled,false) == open-single-summary.html disabled-single-summary-ref.html
+pref(dom.details_element.enabled,false) == no-summary.html disabled-no-summary-ref.html
+
 # Basic <summary> handling
 == multiple-summary.html single-summary.html
 == open-multiple-summary.html open-multiple-summary-ref.html

layout/style/nsLayoutStylesheetCache.cpp

@@ -121,6 +121,11 @@ nsLayoutStylesheetCache::UASheet()
 StyleSheet*
 nsLayoutStylesheetCache::HTMLSheet()
 {
+  if (!mHTMLSheet) {
+    LoadSheetURL("resource://gre-resources/html.css",
+                 &mHTMLSheet, eAgentSheetFeatures, eCrash);
+  }
+
   return mHTMLSheet;
 }
 
@@ -316,8 +321,6 @@ nsLayoutStylesheetCache::nsLayoutStylesheetCache(StyleBackendType aType)
   // per-profile, since they're profile-invariant.
   LoadSheetURL("resource://gre-resources/counterstyles.css",
                &mCounterStylesSheet, eAgentSheetFeatures, eCrash);
-  LoadSheetURL("resource://gre-resources/html.css",
-               &mHTMLSheet, eAgentSheetFeatures, eCrash);
   LoadSheetURL("chrome://global/content/minimal-xul.css",
                &mMinimalXULSheet, eAgentSheetFeatures, eCrash);
   LoadSheetURL("resource://gre-resources/quirk.css",
@@ -378,6 +381,8 @@ nsLayoutStylesheetCache::For(StyleBackendType aType)
     //                               "layout.css.example-pref.enabled");
     Preferences::RegisterCallback(&DependentPrefChanged,
                                   "layout.css.grid.enabled");
+    Preferences::RegisterCallback(&DependentPrefChanged,
+                                  "dom.details_element.enabled");
   }
 
   return cache;
@@ -549,6 +554,7 @@ nsLayoutStylesheetCache::DependentPrefChanged(const char* aPref, void* aData)
                   gStyleCache_Servo ? &gStyleCache_Servo->sheet_ : nullptr);
 
   INVALIDATE(mUASheet);  // for layout.css.grid.enabled
+  INVALIDATE(mHTMLSheet); // for dom.details_element.enabled
 
 #undef INVALIDATE
 }

modules/libpref/init/all.js

@@ -5106,6 +5106,9 @@ pref("dom.audiochannel.mutedByDefault", false);
 // HTML <dialog> element
 pref("dom.dialog_element.enabled", false);
 
+// Enable <details> and <summary> tags.
+pref("dom.details_element.enabled", true);
+
 // Secure Element API
 #ifdef MOZ_SECUREELEMENT
 pref("dom.secureelement.enabled", false);

testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/details.html.ini

@@ -0,0 +1,3 @@
+[details.html]
+  type: testharness
+  prefs: [dom.details_element.enabled:true]

testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/toggleEvent.html.ini

@@ -0,0 +1,3 @@
+[toggleEvent.html]
+  type: testharness
+  prefs: [dom.details_element.enabled:true]