Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Pistonight opened this issue Oct 28, 2024 · 2 comments · Fixed by #2463
Closed

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

Pistonight opened this issue Oct 28, 2024 · 2 comments · Fixed by #2463
Labels
C-bug Category: A bug, incorrect or unintended behavior

Comments

@Pistonight
Copy link
Contributor

Pistonight commented Oct 28, 2024

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 Pistonight added the C-bug Category: A bug, incorrect or unintended behavior label Oct 28, 2024
@Pistonight
Copy link
Contributor Author

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

@ehuss
Copy link
Contributor

ehuss commented Oct 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: A bug, incorrect or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants