Skip to content

Multiple books on the same host contaminate each other #455

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

Open
Michael-F-Bryan opened this issue Sep 30, 2017 · 7 comments
Open

Multiple books on the same host contaminate each other #455

Michael-F-Bryan opened this issue Sep 30, 2017 · 7 comments
Labels
A-JavaScript Area: Javascript C-papercut Category: A small usability bug M-Discussion Meta: Discussion

Comments

@Michael-F-Bryan
Copy link
Contributor

If multiple books are on the same host, changing the theme on one can magically change the theme on all the others.

Steps to Reproduce:

  1. Open up the first and second versions of the Rust Book in separate tabs
  2. Go to the first book and change its theme
  3. Refresh the second book's tab

You'll probably also see this if you use mdbook on multiple projects and have them hosted on GitHub Pages.

This is somewhat related to rust-lang/rust#44704 (see @steveklabnik's comment) in that local storage keys aren't handled as well as they could be.

mdbook stores the current theme in localstorage. This did change in newer/older mdbook, but it looks like this, basically:

   var theme = localStorage.getItem('theme');
   if (theme === null) { theme = 'light'; }

Ideally instead of using a key like "theme", it'd be namespaced. So in the case of the first Rust Book you might use "/book/first-edition:theme".

There may also be other cases where local storage handling allows cross-book contamination.

@projektir
Copy link
Contributor

If multiple books are on the same host, changing the theme on one can magically change the theme on all the others.

Bug or feature? 😀

@azerupi
Copy link
Contributor

azerupi commented Sep 30, 2017

Indeed, for some this can be a feature and for others this can be a bug. I am not sure what behavior we should adopt. If we conclude this is a bug, then prefixing all keys with the book's title should be enough.

@Michael-F-Bryan
Copy link
Contributor Author

It might be worth asking someone on the Rust docs team, seeing as they're the group most affected by this.

From a purity perspective I'd call this a bug. It's pretty much a web version of global mutable state and data races.

@budziq
Copy link
Contributor

budziq commented Sep 30, 2017

It might be nice for people wanting to set coherent dark theme everywhere but its unexpectedly shared between domains. Moreover, not all mdBook clients are related to docs team or rust at all. So certainly a bug!

@azerupi
Copy link
Contributor

azerupi commented Sep 30, 2017

shared between domains

I doubt that, localstorage, or any other web storage, is scoped per domain as far as I know. I am not sure how sub-domains are handled though.

@budziq
Copy link
Contributor

budziq commented Sep 30, 2017

Right I forgot about it! Sorry
So yeah only localhost problem on dev environment. But still a bug in my book (although a relatively minor one).

@Michael-F-Bryan
Copy link
Contributor Author

It doesn't just affect dev environments though. For example, I've got multiple repositories which use mdbook and are published to GitHub Pages (e.g. my FFI guide). These will all be affected, likewise anyone else who has multiple books published to GitHub pages will be the same.

And of course, other big case of multiple books on the same domain is the multiple Rust Book versions and the Nomicon.

I agree that books contaminating each others themes is quite a minor issue. Although if it's happened once, chances are other places using local storage will have the same issue.

@azerupi azerupi added A-JavaScript Area: Javascript M-Discussion Meta: Discussion C-papercut Category: A small usability bug labels Oct 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-JavaScript Area: Javascript C-papercut Category: A small usability bug M-Discussion Meta: Discussion
Projects
None yet
Development

No branches or pull requests

4 participants