From 9d554281e059196661f4dbb44e99f2eff9bfd381 Mon Sep 17 00:00:00 2001
From: Kira Bruneau <kira.bruneau@pm.me>
Date: Wed, 19 Mar 2025 14:37:13 -0400
Subject: [PATCH] firefox: refactor bookmarks into a submodule & require force
 (#6402)

This splits the bookmarks submodule into a seperate file, to make it easier to maintain (like how the search module was previously split out in #5697).

This also refactors bookmarks to require a new force option, to be more explicit about overriding existing bookmarks.
---
 modules/programs/firefox/mkFirefoxModule.nix  | 224 +++++-------------
 .../programs/firefox/profiles/bookmarks.nix   | 203 ++++++++++++++++
 .../firefox/profiles/bookmarks/default.nix    | 104 ++++----
 .../bookmarks/expected-bookmarks-user.js      |   2 +-
 .../programs/firefox/profiles/shared-path.nix |  15 +-
 5 files changed, 317 insertions(+), 231 deletions(-)
 create mode 100644 modules/programs/firefox/profiles/bookmarks.nix

diff --git a/modules/programs/firefox/mkFirefoxModule.nix b/modules/programs/firefox/mkFirefoxModule.nix
index 733c8ec94..26f9046b3 100644
--- a/modules/programs/firefox/mkFirefoxModule.nix
+++ b/modules/programs/firefox/mkFirefoxModule.nix
@@ -56,10 +56,10 @@ let
     else
       builtins.toJSON pref);
 
-  mkUserJs = prePrefs: prefs: extraPrefs: bookmarks: extensions:
+  mkUserJs = prePrefs: prefs: extraPrefs: bookmarksFile: extensions:
     let
-      prefs' = lib.optionalAttrs ([ ] != bookmarks) {
-        "browser.bookmarks.file" = toString (browserBookmarksFile bookmarks);
+      prefs' = lib.optionalAttrs (bookmarksFile != null) {
+        "browser.bookmarks.file" = toString bookmarksFile;
         "browser.places.importBookmarksHTML" = true;
       } // lib.optionalAttrs (extensions != { }) {
         "extensions.webextensions.ExtensionStorageIDB.enabled" = false;
@@ -112,59 +112,6 @@ let
       }}
     '';
 
-  browserBookmarksFile = bookmarks:
-    let
-      indent = level:
-        lib.concatStringsSep "" (map (lib.const "  ") (lib.range 1 level));
-
-      bookmarkToHTML = indentLevel: bookmark:
-        ''
-          ${indent indentLevel}<DT><A HREF="${
-            escapeXML bookmark.url
-          }" ADD_DATE="1" LAST_MODIFIED="1"${
-            lib.optionalString (bookmark.keyword != null)
-            " SHORTCUTURL=\"${escapeXML bookmark.keyword}\""
-          }${
-            lib.optionalString (bookmark.tags != [ ])
-            " TAGS=\"${escapeXML (concatStringsSep "," bookmark.tags)}\""
-          }>${escapeXML bookmark.name}</A>'';
-
-      directoryToHTML = indentLevel: directory: ''
-        ${indent indentLevel}<DT>${
-          if directory.toolbar then
-            ''
-              <H3 ADD_DATE="1" LAST_MODIFIED="1" PERSONAL_TOOLBAR_FOLDER="true">Bookmarks Toolbar''
-          else
-            ''<H3 ADD_DATE="1" LAST_MODIFIED="1">${escapeXML directory.name}''
-        }</H3>
-        ${indent indentLevel}<DL><p>
-        ${allItemsToHTML (indentLevel + 1) directory.bookmarks}
-        ${indent indentLevel}</DL><p>'';
-
-      itemToHTMLOrRecurse = indentLevel: item:
-        if item ? "url" then
-          bookmarkToHTML indentLevel item
-        else
-          directoryToHTML indentLevel item;
-
-      allItemsToHTML = indentLevel: bookmarks:
-        lib.concatStringsSep "\n"
-        (map (itemToHTMLOrRecurse indentLevel) bookmarks);
-
-      bookmarkEntries = allItemsToHTML 1 bookmarks;
-    in pkgs.writeText "${packageName}-bookmarks.html" ''
-      <!DOCTYPE NETSCAPE-Bookmark-file-1>
-      <!-- This is an automatically generated file.
-        It will be read and overwritten.
-        DO NOT EDIT! -->
-      <META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=UTF-8">
-      <TITLE>Bookmarks</TITLE>
-      <H1>Bookmarks Menu</H1>
-      <DL><p>
-      ${bookmarkEntries}
-      </DL>
-    '';
-
   mkNoDuplicateAssertion = entities: entityKind:
     (let
       # Return an attribute set with entity IDs as keys and a list of
@@ -334,6 +281,8 @@ in {
     profiles = mkOption {
       inherit visible;
       type = types.attrsOf (types.submodule ({ config, name, ... }: {
+        imports = [ (pkgs.path + "/nixos/modules/misc/assertions.nix") ];
+
         options = {
           name = mkOption {
             type = types.str;
@@ -430,104 +379,32 @@ in {
           };
 
           bookmarks = mkOption {
+            type = (with types;
+              coercedTo (listOf anything) (bookmarks:
+                warn ''
+                  ${cfg.name} bookmarks have been refactored into a submodule that now explicitly require a 'force' option to be enabled.
+
+                  Replace:
+
+                  ${moduleName}.profiles.${name}.bookmarks = [ ... ];
+
+                  With:
+
+                  ${moduleName}.profiles.${name}.bookmarks = {
+                    force = true;
+                    settings = [ ... ];
+                  };
+                '' {
+                  force = true;
+                  settings = bookmarks;
+                }) (submodule ({ config, ... }:
+                  import ./profiles/bookmarks.nix {
+                    inherit config lib pkgs;
+                    modulePath = modulePath ++ [ "profiles" name "bookmarks" ];
+                  })));
+            default = { };
             internal = !enableBookmarks;
-            type = let
-              bookmarkSubmodule = types.submodule ({ config, name, ... }: {
-                options = {
-                  name = mkOption {
-                    type = types.str;
-                    default = name;
-                    description = "Bookmark name.";
-                  };
-
-                  tags = mkOption {
-                    type = types.listOf types.str;
-                    default = [ ];
-                    description = "Bookmark tags.";
-                  };
-
-                  keyword = mkOption {
-                    type = types.nullOr types.str;
-                    default = null;
-                    description = "Bookmark search keyword.";
-                  };
-
-                  url = mkOption {
-                    type = types.str;
-                    description = "Bookmark url, use %s for search terms.";
-                  };
-                };
-              }) // {
-                description = "bookmark submodule";
-              };
-
-              bookmarkType = types.addCheck bookmarkSubmodule (x: x ? "url");
-
-              directoryType = types.submodule ({ config, name, ... }: {
-                options = {
-                  name = mkOption {
-                    type = types.str;
-                    default = name;
-                    description = "Directory name.";
-                  };
-
-                  bookmarks = mkOption {
-                    type = types.listOf nodeType;
-                    default = [ ];
-                    description = "Bookmarks within directory.";
-                  };
-
-                  toolbar = mkOption {
-                    type = types.bool;
-                    default = false;
-                    description = ''
-                      Make this the toolbar directory. Note, this does _not_
-                      mean that this directory will be added to the toolbar,
-                      this directory _is_ the toolbar.
-                    '';
-                  };
-                };
-              }) // {
-                description = "directory submodule";
-              };
-
-              nodeType = types.either bookmarkType directoryType;
-            in with types;
-            coercedTo (attrsOf nodeType) attrValues (listOf nodeType);
-            default = [ ];
-            example = literalExpression ''
-              [
-                {
-                  name = "wikipedia";
-                  tags = [ "wiki" ];
-                  keyword = "wiki";
-                  url = "https://en.wikipedia.org/wiki/Special:Search?search=%s&go=Go";
-                }
-                {
-                  name = "kernel.org";
-                  url = "https://www.kernel.org";
-                }
-                {
-                  name = "Nix sites";
-                  toolbar = true;
-                  bookmarks = [
-                    {
-                      name = "homepage";
-                      url = "https://nixos.org/";
-                    }
-                    {
-                      name = "wiki";
-                      tags = [ "wiki" "nix" ];
-                      url = "https://wiki.nixos.org/";
-                    }
-                  ];
-                }
-              ]
-            '';
-            description = ''
-              Preloaded bookmarks. Note, this may silently overwrite any
-              previously existing bookmarks!
-            '';
+            description = "Declarative bookmarks.";
           };
 
           path = mkOption {
@@ -763,6 +640,26 @@ in {
             '';
           };
         };
+
+        config = {
+          assertions = [
+            (mkNoDuplicateAssertion config.containers "container")
+            {
+              assertion = config.extensions.settings == { }
+                || config.extensions.force;
+              message = ''
+                Using '${
+                  lib.showAttrPath (modulePath
+                    ++ [ "profiles" profileName "extensions" "settings" ])
+                }' will override all previous extensions settings.
+                Enable '${
+                  lib.showAttrPath (modulePath
+                    ++ [ "profiles" profileName "extensions" "force" ])
+                }' to acknowledge this.
+              '';
+            }
+          ] ++ config.bookmarks.assertions;
+        };
       }));
       default = { };
       description = "Attribute set of ${appName} profiles.";
@@ -826,22 +723,7 @@ in {
       }
 
       (mkNoDuplicateAssertion cfg.profiles "profile")
-    ] ++ (mapAttrsToList
-      (_: profile: mkNoDuplicateAssertion profile.containers "container")
-      cfg.profiles) ++ (mapAttrsToList (profileName: profile: {
-        assertion = profile.extensions.settings == { }
-          || profile.extensions.force;
-        message = ''
-          Using '${
-            lib.showAttrPath
-            (modulePath ++ [ "profiles" profileName "extensions" "settings" ])
-          }' will override all previous extensions settings.
-          Enable '${
-            lib.showAttrPath
-            (modulePath ++ [ "profiles" profileName "extensions" "force" ])
-          }' to acknowledge this.
-        '';
-      }) cfg.profiles);
+    ] ++ (concatMap (profile: profile.assertions) (attrValues cfg.profiles));
 
     warnings = optional (cfg.enableGnomeExtensions or false) ''
       Using '${moduleName}.enableGnomeExtensions' has been deprecated and
@@ -883,10 +765,10 @@ in {
 
         "${profilesPath}/${profile.path}/user.js" = mkIf (profile.preConfig
           != "" || profile.settings != { } || profile.extraConfig != ""
-          || profile.bookmarks != [ ]) {
+          || profile.bookmarks.configFile != null) {
             text =
               mkUserJs profile.preConfig profile.settings profile.extraConfig
-              profile.bookmarks profile.extensions.settings;
+              profile.bookmarks.configFile profile.extensions.settings;
           };
 
         "${profilesPath}/${profile.path}/containers.json" =
diff --git a/modules/programs/firefox/profiles/bookmarks.nix b/modules/programs/firefox/profiles/bookmarks.nix
new file mode 100644
index 000000000..469d543b0
--- /dev/null
+++ b/modules/programs/firefox/profiles/bookmarks.nix
@@ -0,0 +1,203 @@
+{ config, lib, pkgs, modulePath }:
+
+with lib;
+
+let
+  bookmarkSubmodule = types.submodule ({ name, ... }: {
+    options = {
+      name = mkOption {
+        type = types.str;
+        default = name;
+        description = "Bookmark name.";
+      };
+
+      tags = mkOption {
+        type = types.listOf types.str;
+        default = [ ];
+        description = "Bookmark tags.";
+      };
+
+      keyword = mkOption {
+        type = types.nullOr types.str;
+        default = null;
+        description = "Bookmark search keyword.";
+      };
+
+      url = mkOption {
+        type = types.str;
+        description = "Bookmark url, use %s for search terms.";
+      };
+    };
+  }) // {
+    description = "bookmark submodule";
+  };
+
+  bookmarkType = types.addCheck bookmarkSubmodule (x: x ? "url");
+
+  directoryType = types.submodule ({ name, ... }: {
+    options = {
+      name = mkOption {
+        type = types.str;
+        default = name;
+        description = "Directory name.";
+      };
+
+      bookmarks = mkOption {
+        type = types.listOf nodeType;
+        default = [ ];
+        description = "Bookmarks within directory.";
+      };
+
+      toolbar = mkOption {
+        type = types.bool;
+        default = false;
+        description = ''
+          Make this the toolbar directory. Note, this does _not_
+          mean that this directory will be added to the toolbar,
+          this directory _is_ the toolbar.
+        '';
+      };
+    };
+  }) // {
+    description = "directory submodule";
+  };
+
+  nodeType = types.either bookmarkType directoryType;
+
+  bookmarksFile = bookmarks:
+    let
+      indent = level:
+        lib.concatStringsSep "" (map (lib.const "  ") (lib.range 1 level));
+
+      bookmarkToHTML = indentLevel: bookmark:
+        ''
+          ${indent indentLevel}<DT><A HREF="${
+            escapeXML bookmark.url
+          }" ADD_DATE="1" LAST_MODIFIED="1"${
+            lib.optionalString (bookmark.keyword != null)
+            " SHORTCUTURL=\"${escapeXML bookmark.keyword}\""
+          }${
+            lib.optionalString (bookmark.tags != [ ])
+            " TAGS=\"${escapeXML (concatStringsSep "," bookmark.tags)}\""
+          }>${escapeXML bookmark.name}</A>'';
+
+      directoryToHTML = indentLevel: directory: ''
+        ${indent indentLevel}<DT>${
+          if directory.toolbar then
+            ''
+              <H3 ADD_DATE="1" LAST_MODIFIED="1" PERSONAL_TOOLBAR_FOLDER="true">Bookmarks Toolbar''
+          else
+            ''<H3 ADD_DATE="1" LAST_MODIFIED="1">${escapeXML directory.name}''
+        }</H3>
+        ${indent indentLevel}<DL><p>
+        ${allItemsToHTML (indentLevel + 1) directory.bookmarks}
+        ${indent indentLevel}</DL><p>'';
+
+      itemToHTMLOrRecurse = indentLevel: item:
+        if item ? "url" then
+          bookmarkToHTML indentLevel item
+        else
+          directoryToHTML indentLevel item;
+
+      allItemsToHTML = indentLevel: bookmarks:
+        lib.concatStringsSep "\n"
+        (map (itemToHTMLOrRecurse indentLevel) bookmarks);
+
+      bookmarkEntries = allItemsToHTML 1 bookmarks;
+    in pkgs.writeText "bookmarks.html" ''
+      <!DOCTYPE NETSCAPE-Bookmark-file-1>
+      <!-- This is an automatically generated file.
+        It will be read and overwritten.
+        DO NOT EDIT! -->
+      <META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=UTF-8">
+      <TITLE>Bookmarks</TITLE>
+      <H1>Bookmarks Menu</H1>
+      <DL><p>
+      ${bookmarkEntries}
+      </DL>
+    '';
+in {
+  imports = [
+    (pkgs.path + "/nixos/modules/misc/assertions.nix")
+    (pkgs.path + "/nixos/modules/misc/meta.nix")
+  ];
+
+  # We're currently looking for a maintainer who actively uses bookmarks!
+  meta.maintainers = with maintainers; [ kira-bruneau ];
+
+  options = {
+    enable = mkOption {
+      type = with types; bool;
+      default = config.settings != [ ];
+      internal = true;
+    };
+
+    force = mkOption {
+      type = with types; bool;
+      default = false;
+      description = ''
+        Whether to force override existing custom bookmarks.
+      '';
+    };
+
+    settings = mkOption {
+      type = with types;
+        coercedTo (attrsOf nodeType) attrValues (listOf nodeType);
+      default = [ ];
+      example = literalExpression ''
+        [
+          {
+            name = "wikipedia";
+            tags = [ "wiki" ];
+            keyword = "wiki";
+            url = "https://en.wikipedia.org/wiki/Special:Search?search=%s&go=Go";
+          }
+          {
+            name = "kernel.org";
+            url = "https://www.kernel.org";
+          }
+          {
+            name = "Nix sites";
+            toolbar = true;
+            bookmarks = [
+              {
+                name = "homepage";
+                url = "https://nixos.org/";
+              }
+              {
+                name = "wiki";
+                tags = [ "wiki" "nix" ];
+                url = "https://wiki.nixos.org/";
+              }
+            ];
+          }
+        ]
+      '';
+      description = ''
+        Custom bookmarks.
+      '';
+    };
+
+    configFile = mkOption {
+      type = with types; nullOr path;
+      default = if config.enable then bookmarksFile config.settings else null;
+      description = ''
+        Configuration file to define custom bookmarks.
+      '';
+    };
+  };
+
+  config = {
+    assertions = [{
+      assertion = config.enable -> config.force;
+      message = ''
+        Using '${
+          lib.showAttrPath (modulePath ++ [ "settings" ])
+        }' will override all previous bookmarks.
+        Enable ${
+          lib.showAttrPath (modulePath ++ [ "force" ])
+        }' to acknowledge this.
+      '';
+    }];
+  };
+}
diff --git a/tests/modules/programs/firefox/profiles/bookmarks/default.nix b/tests/modules/programs/firefox/profiles/bookmarks/default.nix
index 81766719e..2d647d038 100644
--- a/tests/modules/programs/firefox/profiles/bookmarks/default.nix
+++ b/tests/modules/programs/firefox/profiles/bookmarks/default.nix
@@ -7,12 +7,6 @@ let
 
   firefoxMockOverlay = import ../../setup-firefox-mock-overlay.nix modulePath;
 
-  withName = path:
-    pkgs.substituteAll {
-      src = path;
-      name = cfg.wrappedPackageName;
-    };
-
 in {
   imports = [ firefoxMockOverlay ];
 
@@ -20,52 +14,56 @@ in {
     enable = true;
     profiles.bookmarks = {
       settings = { "general.smoothScroll" = false; };
-      bookmarks = [
-        {
-          toolbar = true;
-          bookmarks = [{
-            name = "Home Manager";
-            url = "https://wiki.nixos.org/wiki/Home_Manager";
-          }];
-        }
-        {
-          name = "wikipedia";
-          tags = [ "wiki" ];
-          keyword = "wiki";
-          url = "https://en.wikipedia.org/wiki/Special:Search?search=%s&go=Go";
-        }
-        {
-          name = "kernel.org";
-          url = "https://www.kernel.org";
-        }
-        {
-          name = "Nix sites";
-          bookmarks = [
-            {
-              name = "homepage";
-              url = "https://nixos.org/";
-            }
-            {
-              name = "wiki";
-              tags = [ "wiki" "nix" ];
-              url = "https://wiki.nixos.org/";
-            }
-            {
-              name = "Nix sites";
-              bookmarks = [
-                {
-                  name = "homepage";
-                  url = "https://nixos.org/";
-                }
-                {
-                  name = "wiki";
-                  url = "https://wiki.nixos.org/";
-                }
-              ];
-            }
-          ];
-        }
-      ];
+      bookmarks = {
+        force = true;
+        settings = [
+          {
+            toolbar = true;
+            bookmarks = [{
+              name = "Home Manager";
+              url = "https://wiki.nixos.org/wiki/Home_Manager";
+            }];
+          }
+          {
+            name = "wikipedia";
+            tags = [ "wiki" ];
+            keyword = "wiki";
+            url =
+              "https://en.wikipedia.org/wiki/Special:Search?search=%s&go=Go";
+          }
+          {
+            name = "kernel.org";
+            url = "https://www.kernel.org";
+          }
+          {
+            name = "Nix sites";
+            bookmarks = [
+              {
+                name = "homepage";
+                url = "https://nixos.org/";
+              }
+              {
+                name = "wiki";
+                tags = [ "wiki" "nix" ];
+                url = "https://wiki.nixos.org/";
+              }
+              {
+                name = "Nix sites";
+                bookmarks = [
+                  {
+                    name = "homepage";
+                    url = "https://nixos.org/";
+                  }
+                  {
+                    name = "wiki";
+                    url = "https://wiki.nixos.org/";
+                  }
+                ];
+              }
+            ];
+          }
+        ];
+      };
     };
   } // {
     nmt.script = ''
@@ -74,7 +72,7 @@ in {
 
       assertFileContent \
         $bookmarksUserJs \
-        ${withName ./expected-bookmarks-user.js}
+        ${./expected-bookmarks-user.js}
 
       bookmarksFile="$(sed -n \
         '/browser.bookmarks.file/ {s|^.*\(/nix/store[^"]*\).*|\1|;p}' \
diff --git a/tests/modules/programs/firefox/profiles/bookmarks/expected-bookmarks-user.js b/tests/modules/programs/firefox/profiles/bookmarks/expected-bookmarks-user.js
index d36dccfdf..fe8e8b2eb 100644
--- a/tests/modules/programs/firefox/profiles/bookmarks/expected-bookmarks-user.js
+++ b/tests/modules/programs/firefox/profiles/bookmarks/expected-bookmarks-user.js
@@ -2,7 +2,7 @@
 
 
 
-user_pref("browser.bookmarks.file", "/nix/store/00000000000000000000000000000000-@name@-bookmarks.html");
+user_pref("browser.bookmarks.file", "/nix/store/00000000000000000000000000000000-bookmarks.html");
 user_pref("browser.places.importBookmarksHTML", true);
 user_pref("general.smoothScroll", false);
 
diff --git a/tests/modules/programs/firefox/profiles/shared-path.nix b/tests/modules/programs/firefox/profiles/shared-path.nix
index b0936bfaa..5c0186727 100644
--- a/tests/modules/programs/firefox/profiles/shared-path.nix
+++ b/tests/modules/programs/firefox/profiles/shared-path.nix
@@ -12,13 +12,16 @@ in {
       main = {
         isDefault = true;
         id = 1;
-        bookmarks = [{
-          toolbar = true;
-          bookmarks = [{
-            name = "Home Manager";
-            url = "https://wiki.nixos.org/wiki/Home_Manager";
+        bookmarks = {
+          force = true;
+          settings = [{
+            toolbar = true;
+            bookmarks = [{
+              name = "Home Manager";
+              url = "https://wiki.nixos.org/wiki/Home_Manager";
+            }];
           }];
-        }];
+        };
         containers = {
           "shopping" = {
             icon = "circle";