[FIX] Use logo url as is to allow for web urls.

[feanil]

logo_url can sometimes be a fully qualified URL starting in Sphinx 4.
Adding the _static prefix breaks logos that are URLs.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[choldgraf]

Thanks for the PR - one question, won't this break local paths because we're removing the pathto? It seems we should instead want to use pathto if the variable doesn't start with http.

[feanil]

@choldgraf the HTML builder handles this in Sphinx 4+

master: https://github.com/sphinx-doc/sphinx/blob/master/sphinx/builders/html/__init__.py#L1254-L1255
v4.0.0: https://github.com/sphinx-doc/sphinx/blob/v4.0.0/sphinx/builders/html/__init__.py#L1191-L1195

[pradyunsg]

Why is this being escaped? Isn't it an already-encoded URL?

[feanil]

@pradyunsg I don't think that's guaranteed since the logo_url value comes from the user in conf.py and the code I linked to above doesn't seem to be escaping anything.

[pradyunsg]

You're correct and I have to go fix something in Furo. :)

x-ref: https://github.com/sphinx-doc/sphinx/blob/6e6b9ad45194177811551c77f5f4040213d1c661/sphinx/themes/basic/layout.html#L56

[pradyunsg]

x-ref pradyunsg/furo#253 where I'd discovered this as well. :)

[choldgraf]

Many thanks for this fix and for linking to the sphinx code where this is now handled! :-)

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️

[feanil]

Thanks for the quick reviews @choldgraf and @pradyunsg, when do you guys think this will be released in a new version?

[choldgraf]

There's a release candidate out now and this will make it into the next RC i think

[melund]

@fenil/@choldgraf The master version of sphinx-book-theme doesn't use the sidebar-logo.htm anymore, but instead the navbar-logo.html from the pydata theme.

That version does check that the if the string is an URL, but if not it prepends _static/ which causes me to see it prepended twice.

A workaround is to specify the logo using the PyData logo config (light and dark theme). I guess that prevents sphinx from also prepending the _static:

html_theme_options: Dict[str, Any] = {
      ...,
    "logo": {
      "image_light": "logo.svg",
      "image_dark": "logo.svg",
   },

[feanil]

@melund it sounds like maybe this same fix needs to be made in navbar-logo.html as well?