Fix accidental disabling of forward-declared stylesheets (just-the-do…

…cs#1373) PR for the issue flagged in just-the-docs#1359 (reply in thread) (and related to just-the-docs#1358, just-the-docs#1350, etc.). This PR essentially just disables the `head-nav` stylesheet by ID instead of relying on the index; this makes the code more robust against stylesheets that are arbitrarily entered into the document (such as by JavaScript, which is the case of just-the-docs#1359).
pdmosses · Oct 25, 2023 · 80bd7bf · 80bd7bf
1 parent 56e0f1c
commit 80bd7bf
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ Code changes to `main` that are *not* in the latest release:
 - Fixed: erroneous parentheses in `site_nav` conditional by [@mattxwang] in [#1366]
 - Fixed: navigation scroll to active link regression by [@pdmosses] in [#1367]
 - Fixed: invalid CSS rules in head elements by [@pdmosses] in [#1368]
+- Fixed: accidental disabling of forward-declared stylesheets by [@mattxwang] in [#1373]
 
 {: .warning }
 [#1358] moved `_includes/nav.html` to the `_includes/components` directory,
@@ -51,6 +52,7 @@ Users who were overriding that file will need to adjust their sites accordingly.
 [#1366]: https://github.com/just-the-docs/just-the-docs/pull/1366
 [#1367]: https://github.com/just-the-docs/just-the-docs/pull/1367
 [#1368]: https://github.com/just-the-docs/just-the-docs/pull/1368
+[#1373]: https://github.com/just-the-docs/just-the-docs/pull/1373
 [#1377]: https://github.com/just-the-docs/just-the-docs/pull/1377
 
 ## Release v0.6.2

diff --git a/_includes/head.html b/_includes/head.html
@@ -5,7 +5,7 @@
   Results in: HTML for the head element.
   Includes:
     css/activation.scss.liquid, head_custom.html.
-  Overwrites: 
+  Overwrites:
     ga_tracking_ids, ga_property, file, favicon.
   Should not be cached, because included files depend on page.
 {%- endcomment -%}
@@ -15,9 +15,9 @@
   <meta http-equiv="X-UA-Compatible" content="IE=Edge">
 
   <link rel="stylesheet" href="{{ '/assets/css/just-the-docs-default.css' | relative_url }}">
-  
-  <link rel="stylesheet" href="{{ '/assets/css/just-the-docs-head-nav.css' | relative_url }}">
-  
+
+  <link rel="stylesheet" href="{{ '/assets/css/just-the-docs-head-nav.css' | relative_url }}" id="jtd-head-nav-stylesheet">
+
   <style id="jtd-nav-activation">
   {% include css/activation.scss.liquid %}
   </style>

diff --git a/assets/js/just-the-docs.js b/assets/js/just-the-docs.js
@@ -38,7 +38,7 @@ function initNav() {
   const siteNav = document.getElementById('site-nav');
   const mainHeader = document.getElementById('main-header');
   const menuButton = document.getElementById('menu-button');
-  
+
   disableHeadStyleSheets();
 
   jtd.addEvent(menuButton, 'click', function(e){
@@ -69,15 +69,19 @@ function initNav() {
 }
 
 // The <head> element is assumed to include the following stylesheets:
-// 0. a <link> to /assets/css/just-the-docs-default.css
-// 1. a <link> to /assets/css/just-the-docs-head-nav.css
-// 2. a <style> containing the result of _includes/css/activation.scss.liquid.
-// It also includes any styles provided by users in _includes/head_custom.html.
-// Stylesheet 2 may be missing (compression can remove empty <style> elements)
-// so disableHeadStyleSheet() needs to access it by its id.
+// - a <link> to /assets/css/just-the-docs-head-nav.css,
+//             with id 'jtd-head-nav-stylesheet'
+// - a <style> containing the result of _includes/css/activation.scss.liquid.
+// To avoid relying on the order of stylesheets (which can change with HTML
+// compression, user-added JavaScript, and other side effects), stylesheets
+// are only interacted with via ID
 
 function disableHeadStyleSheets() {
-  document.styleSheets[1].disabled = true;
+  const headNav = document.getElementById('jtd-head-nav-stylesheet');
+  if (headNav) {
+    headNav.disabled = true;
+  }
+
   const activation = document.getElementById('jtd-nav-activation');
   if (activation) {
     activation.disabled = true;