search

readthedocs / sphinx-notfound-page

Create a custom 404 page with absolute URLs hardcoded
https://sphinx-notfound-page.readthedocs.io/
MIT License
50 stars 32 forks source link

Override css_tag method #120

Closed samscott89 closed 3 years ago

samscott89 commented 3 years ago

I was having an issue where stylesheets added with add_css_file weren't being handled correctly, since they go through a hardcoded pathto in sphinx, instead of the ctx['pathto'] method.

This fixes the issue, but it might also be better fixed upstream in sphinx? I'm not very familiar with the ecosystem, or what sphinx should be doing here.

humitos commented 3 years ago

Hi @samscott89, thanks for your PR!

I'm not sure to understand what is the issue that this PR fixes. Could you provide a detailed description and maybe an isolated small example project for this that I can use to reproduce it?

Also, would be good if you can write a test case together on this PR so we are sure that we are not hitting this issue again in the future.

samscott89 commented 3 years ago

Hi @samscott89, thanks for your PR!

I'm not sure to understand what is the issue that this PR fixes. Could you provide a detailed description and maybe an isolated small example project for this that I can use to reproduce it?

Also, would be good if you can write a test case together on this PR so we are sure that we are not hitting this issue again in the future.

No problem! Hopefully the latest commit does both in one :) The added test fails against main - the add_css_file (or add_stylesheet) method adds files in a way which skips the pathto method.

So any extensions using that method (or any project) ends up with files that aren't fixed.

humitos commented 3 years ago

If I understand correctly, the work done in this PR is replaced by #153 and it's not required anymore. @samscott89 could you please help me testing that PR with your project and letting me know that it works as you expected?

domdfcoding commented 3 years ago

I ran into this myself, and was working on a fix at https://github.com/sphinx-toolbox/sphinx-notfound-page/tree/css-files before I saw this PR.

What happens is:

When visiting a non-existent page under /en/latest everything works, since the relative target points to the right place. But if the user tries a page elsewhere (such as https://formate.readthedocs.io/usage.html) the relative target is now wrong, and the browser can't load the stylesheets.

In the Firefox console I see errors like: image

Those resources should be loaded from formate.readthedocs.io/en/latest/_static/twemoji.css, etc.

153 might solve this for 4.x (I can't test that with my docs, but looking at the source for https://sphinx-notfound-page.readthedocs.io/usage.html it seems so), but this will still be a problem for Sphinx 3.x

humitos commented 3 years ago

Hi @domdfcoding! You are right! My solution in #153 does not work for older versions of Sphinx.

an extension adds a css file with app.add_css_file("my_style.css"

I wrote a test case for this and I opened PR #156 that fixes this problem and should work for all the Sphinx versions. Would you mind giving it a try and let me know if your issue is solved with it?