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
sphinx/themes/basic/layout: Set url_root properly on index, don't use '#' #8524
sphinx/themes/basic/layout: Set url_root properly on index, don't use '#' #8524
Conversation
It is a bit hidden in there, but this is a full analysis of what goes wrong and why this is needed: executablebooks/sphinx-copybutton#110 |
- You can see a practical demonstration of the problem, fully seeing the effect if it is not fixed, here (though that example uses the alabaster theme, the effect is the same here): executablebooks/sphinx-copybutton#110 - This is a copy of a fix from Sphinx. The sphinx pull request is sphinx-doc/sphinx#8524 Detailed description: - url_root is set to `#` on the index page, which layout.html tries to change back to `''` (the empty string). - But, this updated url_root wasn't used in the actual location, as an argument to `documentation_options.js`. - Thus, clever enough templates, which tried to use `$DOCUMENTATION_OPTIONS.URL_ROOT` inside javascript would fail. This was manifested as broken links, which led to this issue: executablebooks/sphinx-copybutton#110 - I have eventually traced that back to sphinx itself, and found that layout.html tried to fix the problem, but the fixed value wasn't used. - This fix works in my basic test, but I will continue with more tests. - Review: - someone more clever should examine this and make sure it makes sense - This does not have tests. Should it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good. Could you rebase this into the 3.x branch? Then I'll merge this soon.
… '#' - url_root is set to `#` on the index page, which layout.html tries to change back to `''` (the empty string). - But, this updated url_root wasn't used in the actual location, as an argument to `documentation_options.js`. - Thus, clever enough templates, which tried to use `$DOCUMENTATION_OPTIONS.URL_ROOT` inside javascript would fail. This was manifested as broken links, which led to this issue: executablebooks/sphinx-copybutton#110 - I have eventually traced that back to sphinx itself, and found that layout.html tried to fix the problem, but the fixed value wasn't used. - This fix works in my basic test, but I will continue with more tests. - Review: - someone more clever should examine this and make sure it makes sense - This does not have tests. Should it?
313f163
to
21aecb1
Compare
Ah yes, I forgot 3.x is dev. Rebased. Thanks! |
Now PR edited to have a base branch of |
Thank you for your great work! |
#1025) - You can see a practical demonstration of the problem, fully seeing the effect if it is not fixed, here (though that example uses the alabaster theme, the effect is the same here): executablebooks/sphinx-copybutton#110 - This is a copy of a fix from Sphinx. The sphinx pull request is sphinx-doc/sphinx#8524 Detailed description: - url_root is set to `#` on the index page, which layout.html tries to change back to `''` (the empty string). - But, this updated url_root wasn't used in the actual location, as an argument to `documentation_options.js`. - Thus, clever enough templates, which tried to use `$DOCUMENTATION_OPTIONS.URL_ROOT` inside javascript would fail. This was manifested as broken links, which led to this issue: executablebooks/sphinx-copybutton#110 - I have eventually traced that back to sphinx itself, and found that layout.html tried to fix the problem, but the fixed value wasn't used. - This fix works in my basic test, but I will continue with more tests. - Review: - someone more clever should examine this and make sure it makes sense - This does not have tests. Should it?
#
on the index page, which layout.html tries tochange back to
''
(the empty string).argument to
documentation_options.js
.$DOCUMENTATION_OPTIONS.URL_ROOT
inside javascript would fail.This was manifested as broken links, which led to this issue:
copy button <img> doesn't work with dirhtml builder, only on root page executablebooks/sphinx-copybutton#110
layout.html tried to fix the problem, but the fixed value wasn't
used.
sense