Skip to content
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

fix wrong switcher JSON loaded for dev docs #2048

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/user_guide/version-dropdown.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ The switcher requires the following configuration steps:

Below is a more in-depth description of each of these configuration steps.

.. warning::
Modern web browsers do not allow loading data when the request is made from a page loaded via the ``file:`` protocol. This means that if you build your documentation locally and then directly open one of its pages in the browser, **the version switcher will not be allowed to load its JSON data source** and you'll end up with an empty switcher menu. Possible work-arounds are:

1. View the locally-built files through a local webserver (Python provides a builtin module for this: https://docs.python.org/3/library/http.server.html).
2. Disabling your browser's security settings (either by passing a command-line flag when launching the browser, or through a browser add-on).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we print that, or a link to that in the console when we see the file:// protocol ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered doing a console.log but then I thought: pointing folks to this docs page is way easier than explaining how to view browser console. So to me seemed not worth it but I don't mind adding it if you think is

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait I misunderstood you. You're thinking about folks who already are checking browser console when something goes wrong. Yeah, probably worth adding it for their benefit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I meant.

I was also going to point out, if we detect this we could literally fill the version switcher itself with this text; but that may be overkill.


Add a JSON file to define your switcher's versions
--------------------------------------------------
Expand Down
30 changes: 19 additions & 11 deletions src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,23 +406,31 @@ async function checkPageExistsAndRedirect(event) {
* @param {string} url The URL to load version switcher entries from.
*/
async function fetchVersionSwitcherJSON(url) {
const currentPath = getCurrentUrlPath();
// first check if it's a valid URL
try {
var result = new URL(url);
} catch (err) {
if (err instanceof TypeError) {
if (!window.location.origin) {
// window.location.origin is null for local static sites
// (ie. window.location.protocol == 'file:')
//
// TODO: Fix this to return the static version switcher by working out
// how to get the correct path to the switcher JSON file on local static builds
return null;
// Assume we got a relative path, and fix accordingly.
if (window.location.protocol == "file:") {
// Here instead of returning `null` we work out what the file path would be
// anyway (same code path as for served docs), as a convenience to folks who
// routinely disable CORS when they boot up their browser.
console.info(
"[PST] looks like you're viewing this site from a local filesystem, so " +
"the version switcher won't work unless you've disabled CORS. See " +
"https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/version-dropdown.html",
);
}
// assume we got a relative path, and fix accordingly. But first, we need to
// use `fetch()` to follow redirects so we get the correct final base URL
Comment on lines -422 to -423
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment about using fetch() to follow redirects is something I'm now effectively ignoring. @12rambau do you recall the corner case that led to adding this?

const origin = await fetch(window.location.origin, { method: "HEAD" });
result = new URL(url, origin.url);
const cutoff = window.location.href.indexOf(currentPath);
// cutoff == -1 can happen e.g. on the homepage of locally served docs, where you
// get something like http://127.0.0.1:8000/ (no trailing `index.html`)
const origin =
cutoff == -1
? window.location.href
: window.location.href.substring(0, cutoff);
result = new URL(url, origin);
} else {
// something unexpected happened
throw err;
Expand Down
Loading