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

Add favicon.ico to Django static files #229

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

Investigamer
Copy link
Contributor

Description

Fix crash caused by missing favicon.ico static file.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have updated any relevant documentation or created new documentation where appropriate.

Copy link
Collaborator

@ndepaola ndepaola left a comment

Choose a reason for hiding this comment

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

  • the frontend relies on the server having a favicon.ico file for the second step of the server connection workflow (using ping.js):
    image

  • if the system is deployed like you've configured it, where both the frontend and backend are hosted in the same domain, and any requests on /2/ or /admin/ are reverse proxied to django but other requests are reverse proxied to the frontend, then the frontend's favicon.ico will answer the ping request

  • however, if the backend is deployed headlessly (without a frontend) and you try to connect to it through https://mpcautofill.github.io, i expect the ping step will fail because the backend has no favicon.ico

  • so i reckon the correct change here is to revert the deletion of favicon.ico from Delete old frontend #176

Signed-off-by: Investigamer <[email protected]>
@Investigamer Investigamer changed the title Remove unnecessary favicon url which demands staticfile exists. Add favicon.ico to Django static files Mar 26, 2024
Copy link
Contributor Author

@Investigamer Investigamer left a comment

Choose a reason for hiding this comment

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

Done! Looks all set 👍

@ndepaola ndepaola merged commit 4e230e2 into chilli-axe:master Mar 27, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants