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

links on search page are broken when using dirhtml builder #4915

Closed
davidism opened this issue Apr 29, 2018 · 4 comments
Closed

links on search page are broken when using dirhtml builder #4915

davidism opened this issue Apr 29, 2018 · 4 comments

Comments

@davidism
Copy link

davidism commented Apr 29, 2018

#4295 moved the documentation options into a separate JS file. This breaks the search page when using the dirhtml builder because links no longer point to the parent directory, they point below the /search/ page.

Previously, the JS was generated on each page, so pathto('', 1) generated ../ for the search page. However, now that the file is separate which doesn't extend layout.html (since it's not an HTML template), url_root is undefined and the code always generates ''.

Before the change, a link from the search page would look like../api/, so the URL ended up being /api/. Now, it looks like api/, so the URL is /search/api/.

See pallets/flask#2743 for an example of this issue.

@davidism
Copy link
Author

davidism commented Apr 30, 2018

I'm not clear why that was seen as a solution to the CSP error with inline JS, since the search page still contains inline JS.

@davidism
Copy link
Author

My solution for now is to override the extrahead block in layout.html:

{% extends 'basic/layout.html' %}
{% block extrahead %}
<script>DOCUMENTATION_ROOT.URL_ROOT = '{{ url_root }}';</script>
{{ super() }}
{% endblock %}

@ghost
Copy link

ghost commented Apr 30, 2018

Hi, thanks for pointing out the issue. You are correct on two fronts: the non-HTML template expands config variables (but not the built-in functions) and it seems to only get expanded once anyway, so it's not a solution. As for the search page containing inline JS, that needs to be fixed and thanks for catching that. Here's a quick fix:

-  <script type="text/javascript" src="{{ pathto('_static/js/documentation_options.js', 1) }}"></script>
+  <script type="text/javascript" id="documentation_options" data-url_root="{{ pathto('', 1) }}" src="{{ pathto('_static/js/documentation_options.js', 1) }}"></script>
-    URL_ROOT: '{{ url_root }}',
+    URL_ROOT: document.getElementById("documentation_options").getAttribute('data-url_root'),

Let me know if that works for you before I submit a PR.

@tk0miya
Copy link
Member

tk0miya commented May 15, 2018

Fixed by #4916.
Thank you for reporting!

@tk0miya tk0miya closed this as completed May 15, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants