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

Instant Loading: External fetch calls break on Firefox #6565

Closed
4 tasks done
SaltyAimbOtter opened this issue Dec 27, 2023 · 5 comments
Closed
4 tasks done

Instant Loading: External fetch calls break on Firefox #6565

SaltyAimbOtter opened this issue Dec 27, 2023 · 5 comments
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@SaltyAimbOtter
Copy link
Contributor

SaltyAimbOtter commented Dec 27, 2023

Context

We use mkdocs-material to build our project's entire homepage. Therefore we have build a customized page that serves as a download page for our builds.
This is achieved with a small JS script that is added to that specific page and fetches the builds from our nexus repository.

Bug description

When instant loading is enabled, fetch calls to external domains fail in Firefox.
The developer console displays Uncaught (in promise) TypeError: NetworkError when attempting to fetch resource.
The network tab shows NS_BINDING_ABORTED.

The same JS script work fine when instant loading is not enabled.
It always works in Chromium based browsers, no matter if instant loading is enabled.

The only known workaround is to disable instant loading.

Related links

There are four open bugs for instant loading since it is scheduled for a rework.
#6345, #6334, #6275, #6102
None of them specifically adress this issue.

I also though about instant loading's quirk that is described in #5925 and in the docs. I don't think it is related though.

Reproduction

9.5.3+insiders.4.48.0-instant-loading-breaks-fetch-firefox.zip

Steps to reproduce

  1. Setup the reproduction example
  2. Setup a Firefox and a Chromium based browser to work around CORS (our build repo only allows downloads from docs.betonquest.org).
  1. Open the docs index page with Firefox and a Chromium based browser.
  2. Open the developer console in both.
  3. Click the Download button.
  4. Observe the different behaviour where Chromium successfully downloads the builds but Firefox fails.
  5. Disable instant loading
  6. Observe that now both browser are able to download the builds.

Check docs/webCode/download.js, the fetch call couldn't get more vanilla than this. It should definitely work.

Browser

Firefox

Before submitting

SaltyAimbOtter added a commit to SaltyAimbOtter/BetonQuest that referenced this issue Dec 27, 2023
Unfortunately, instant loading breaks fetch calls to external websites in Firefox. See squidfunk/mkdocs-material#6565 for more information.
SaltyAimbOtter added a commit to SaltyAimbOtter/BetonQuest that referenced this issue Dec 27, 2023
Unfortunately, instant loading breaks fetch calls to external websites in Firefox. See squidfunk/mkdocs-material#6565 for more information.
@squidfunk squidfunk added the needs investigation Issue must be investigated by the maintainers label Dec 27, 2023
@squidfunk
Copy link
Owner

Thanks for reporting. Adding event.stopPropagation(); in your event handler fixes the issue. This means that the event is caught and not propagated up the tree where instant loading can catch it. However, what you're doing in your customization actually confuses instant loading, because:

  • You're defining a link with a hash fragment #
  • An event handler is mounted on that link and initiates a download
  • The hash fragment click propagates up the tree where it somehow confuses instant loading

Two solutions:

  1. Use a direct link in Markdown – works perfectly as tested
  2. Add event.stopPropagation() before the fetch call

Closing as resolved by customization. This is not something we're canonically optimizing for. However, if you find the problem in instant loading that fixes the issue, you can create a PR and we can discuss merging it.

@squidfunk squidfunk added resolved by customization Issue can be solved through customization and removed needs investigation Issue must be investigated by the maintainers labels Jan 17, 2024
@squidfunk
Copy link
Owner

squidfunk commented Jan 21, 2024

I've tested this again with the latest refactoring of instant navigation in #6662 and it seems we could fix the reported problem as well! We are still waiting for some users to test drive the latest changes on their projects, so we know that we did not introduce new bugs. We're waiting eagerly to release the fixes ☺️ However, there's one problem: your script gets executed every time you navigate to the page, which will lead to:

Uncaught SyntaxError: Identifier 'latestBuild' has already been declared (at download.js:1:1)

To fix this, you should either put all JavaScript code inside:

document$.subscribe(() => {
  // ... all code goes here
})

... or use var instead of const, which will also make your script work. However, the former approach is definitely recommended, as it is how you integrate with instant navigation. We are working on our documentation to make it more apparent how to integrate third-party JavaScript with instant navigation in the coming months. cc @alexvoss

@squidfunk squidfunk added bug Issue reports a bug resolved Issue is resolved, yet unreleased if open and removed resolved by customization Issue can be solved through customization labels Jan 21, 2024
@squidfunk squidfunk reopened this Jan 21, 2024
@squidfunk
Copy link
Owner

I've reopened the issue to signal that we fixed the bug, and that it's awaiting a release.

@squidfunk
Copy link
Owner

Released as part of 9.5.5.

@SaltyAimbOtter
Copy link
Contributor Author

Hi Martin,
sorry for the late response. I couldn't look into this until now.
Thanks to your fixes and suggestions everything works now! Thanks a lot! ❤️

We do not use the standard Markdown link syntax because there is much more to this setup. I excluded all of it to make your debugging easier. We essentially build a fancy system that polls our Nexus repo for all available dev builds and releases. The user can easily download them on the docs page. Sometimes, the script also needs to rename the downloaded artifact which is why we create the link in that weird way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

No branches or pull requests

2 participants