rust-lang / mdBook

Create book from markdown files. Like Gitbook but implemented in Rust
https://rust-lang.github.io/mdBook/
Mozilla Public License 2.0
18.42k stars 1.65k forks source link

Theme popup not opening when localStorage has an invalid theme id stored #2461

Closed Pistonight closed 3 weeks ago

Pistonight commented 4 weeks ago

Problem

If mdbook-theme in localStorage contains a theme id that's not in the book, an uncaught exception causes theme popup to not function properly

Steps

  1. Create a new project with --theme, build and serve the book
  2. Switch the theme so mdbook-theme gets set in localStorage (may not be needed?)
  3. Edit theme/index.hbs, find <ul id="theme-list" and remove/change the id of the button so that the saved theme is no longer in the list
  4. Rebuild the book
  5. Observe that clicking on the theme button no longer opens the menu

Possible Solution(s)

The root cause is updateThemeSelected() throws if the selected theme is not in the DOM

I think the cleanest fix is to update get_theme() to return default_theme if the saved theme is invalid

    // cache the valid theme ids
    var themeIds = [];
    themePopup.querySelectorAll('button.theme').forEach(function (el) {
        themeIds.push(el.id);
    });

   // ...

    function get_theme() {
        var theme;
        try { theme = localStorage.getItem('mdbook-theme'); } catch (e) { }
        if (theme === null || theme === undefined || !themeIds.includes(theme)) { // <- add this check
            return default_theme;
        } else {
            return theme;
        }
    }

Verified that the solution works

Notes

No response

Version

mdbook v0.4.40

Pistonight commented 4 weeks ago

@ehuss pinging according to the contribution guide. Should I open a PR for this fix?

ehuss commented 3 weeks ago

Seems like a reasonable fix to me, would appreciate a PR!