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.

15 files changed, 111 insertions, 19 deletions

diff --git a/dom/html/HTMLDetailsElement.cpp b/dom/html/HTMLDetailsElement.cpp
index 9d4dd89c2a..74479b8d90 100644
--- a/dom/html/HTMLDetailsElement.cpp
+++ b/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()
 {
 }
diff --git a/dom/html/HTMLDetailsElement.h b/dom/html/HTMLDetailsElement.h
index 4575ed888d..10e70784c6 100644
--- a/dom/html/HTMLDetailsElement.h
+++ b/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)
   {
diff --git a/dom/html/HTMLSummaryElement.cpp b/dom/html/HTMLSummaryElement.cpp
index 42ead6b87f..ee3c07b20b 100644
--- a/dom/html/HTMLSummaryElement.cpp
+++ b/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 {
diff --git a/dom/webidl/EventHandler.webidl b/dom/webidl/EventHandler.webidl
index e7dc4931bd..f7acb66ef4 100644
--- a/dom/webidl/EventHandler.webidl
+++ b/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
diff --git a/dom/webidl/HTMLDetailsElement.webidl b/dom/webidl/HTMLDetailsElement.webidl
index 04df82e30f..0ef20428f5 100644
--- a/dom/webidl/HTMLDetailsElement.webidl
+++ b/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;
diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
index b40e6f8b61..9a0410d737 100644
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/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
diff --git a/layout/generic/crashtests/crashtests.list b/layout/generic/crashtests/crashtests.list
index d44d7614c1..c8fb84feb9 100644
--- a/layout/generic/crashtests/crashtests.list
+++ b/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
-load details-display-none-summary-1.html
-load details-display-none-summary-2.html
-load details-display-none-summary-3.html
-load details-open-overflow-auto.html
-load details-open-overflow-hidden.html
-load details-three-columns.html
+pref(dom.details_element.enabled,true) load details-containing-only-text.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-1.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-2.html
+pref(dom.details_element.enabled,true) load details-display-none-summary-3.html
+pref(dom.details_element.enabled,true) load details-open-overflow-auto.html
+pref(dom.details_element.enabled,true) load details-open-overflow-hidden.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
diff --git a/layout/generic/nsContainerFrame.cpp b/layout/generic/nsContainerFrame.cpp
index 835ba9b262..47afed9de1 100644
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -1891,10 +1891,12 @@ nsContainerFrame::RenumberFrameAndDescendants(int32_t* aOrdinal,
   }
 
   // Do not renumber list for summary elements.
-  HTMLSummaryElement* summary =
-    HTMLSummaryElement::FromContent(kid->GetContent());
-  if (summary && summary->IsMainSummary()) {
-    return false;
+  if (HTMLDetailsElement::IsDetailsEnabled()) {
+    HTMLSummaryElement* summary =
+      HTMLSummaryElement::FromContent(kid->GetContent());
+    if (summary && summary->IsMainSummary()) {
+      return false;
+    }
   }
 
   bool kidRenumberedABullet = false;
diff --git a/layout/reftests/details-summary/disabled-no-summary-ref.html b/layout/reftests/details-summary/disabled-no-summary-ref.html
new file mode 100644
index 0000000000..6ecdbcdc0a
--- /dev/null
+++ b/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>
diff --git a/layout/reftests/details-summary/disabled-single-summary-ref.html b/layout/reftests/details-summary/disabled-single-summary-ref.html
new file mode 100644
index 0000000000..f643af6dcd
--- /dev/null
+++ b/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>
diff --git a/layout/reftests/details-summary/reftest.list b/layout/reftests/details-summary/reftest.list
index a972cf4980..6b558ea137 100644
--- a/layout/reftests/details-summary/reftest.list
+++ b/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
diff --git a/layout/style/nsLayoutStylesheetCache.cpp b/layout/style/nsLayoutStylesheetCache.cpp
index bf87b006ca..1905d8c5cd 100644
--- a/layout/style/nsLayoutStylesheetCache.cpp
+++ b/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
 }
diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
index a24bec7a5a..3a16eef67f 100644
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5098,6 +5098,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);
diff --git a/testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/details.html.ini b/testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/details.html.ini
new file mode 100644
index 0000000000..6ffca742cc
--- /dev/null
+++ b/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]
diff --git a/testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/toggleEvent.html.ini b/testing/web-platform/meta/html/semantics/interactive-elements/the-details-element/toggleEvent.html.ini
new file mode 100644
index 0000000000..335ffd5b3a
--- /dev/null
+++ b/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]