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

/find/ pages are recursive and broken #92

Open
eric-wieser opened this issue Nov 19, 2022 · 10 comments
Open

/find/ pages are recursive and broken #92

eric-wieser opened this issue Nov 19, 2022 · 10 comments

Comments

@eric-wieser
Copy link
Contributor

The page at https://leanprover-community.github.io/mathlib4_docs/find/ohno

gives me

image

@hargoniX
Copy link
Collaborator

hargoniX commented Nov 19, 2022

I'm not quite sure why it is broken like this but this page is not supposed to be visited in the first place, it was changed to be this way: https://leanprover-community.github.io/mathlib4_docs/find/?pattern=Lean.Meta.abstractMVars#doc

@eric-wieser
Copy link
Contributor Author

I think it's broken that way due to use of relative URLs in iframes. If you use absolute urls the issue should go away.

@hargoniX
Copy link
Collaborator

I'm not quite sure how to make the URLs absolute from a conceptual point of view. The idea with the relative URLs in doc-gen4 was to make it so that anyone can put the generated HTML at any sub URL with any prefix (including none) seamlessly. How could we preserve that without relative URLs?

@eric-wieser
Copy link
Contributor Author

I don't think you can solve that. When generating the HTML for the 404 page I think the user needs to specify the folder that the site is hosted from.

If you do that, then you can probably get away with just putting <base href="/$SUB_URL" /> in the 404 page, and leave the rest alone. This seems pretty reasonable, given that 404 pages aren't a standard thing anyway, and doc-gen4 presumably assumes github which probably only serves them from the root directory anyway.

@acmepjz
Copy link
Contributor

acmepjz commented Jun 10, 2024

This https://leanprover-community.github.io/mathlib4_docs/find/?pattern=ohno works. Is there a way to add HTML/javascript code to that page to convert malfotmed https://leanprover-community.github.io/mathlib4_docs/find/ohno to this form?

@hargoniX
Copy link
Collaborator

The issue here is Github Pages AFAICT. I already elaborated this a bit on Zulip but I can repeat what's going on here:

doc-gen already generates a 404.html page that is hosted on Github Pages. That 404.html page heavily relies on being called precisely with /404.html due to relative paths (e.g. it links to style.css at ./style.css. The reason for this relative linking is that doc-gen's pages are supposed to be hostable behind arbitrary prefixes without requiring any intervention from the user.

What happens now is the following:

  1. In the find/?pattern case the JS of find.html notices that it can't find something and does an explicit redirect to /404.html. This makes everything get rendered fine.
  2. In the /find/ohno case, Github Pages decides "aha, I don't have a file that I can present here, I shall instead present 404.html" But it does not perform a redirect to 404.html, instead it just presents the file at this specific path. It will then look for ./style.css (and other files), not find anything and get horribly broken in the way that we see it.

I currently don't really see a nice fix for this, if someone knows a way to make this redirect ourselves or convince Github pages to do it for us I would happily add it.

@acmepjz
Copy link
Contributor

acmepjz commented Jun 10, 2024

Maybe it's better to provide another separate 404 page using absolute path? In fact currently any misspelled page will result in broken 404 page, e.g. https://leanprover-community.github.io/mathlib4_docs/Mathlib/FieldTheory/IntermediateField5.html

@eric-wieser
Copy link
Contributor Author

The reason for this relative linking is that doc-gen's pages are supposed to be hostable behind arbitrary prefixes without requiring any intervention from the user.

I think the thing to do is allow the user to specify a SITE_ROOT that defaults to . (ie the current behavior). In the build for github actions for mathlib, we can use SITE_ROOT=/

@hargoniX
Copy link
Collaborator

The reason for this relative linking is that doc-gen's pages are supposed to be hostable behind arbitrary prefixes without requiring any intervention from the user.

I think the thing to do is allow the user to specify a SITE_ROOT that defaults to . (ie the current behavior). In the build for github actions for mathlib, we can use SITE_ROOT=/

This is the wrong site root though :P the correct one would be /mathlib4_docs/. It used to be the case that doc-gen had this feature but I removed it because people were using it wrongly and got confused. I can get behind the idea of adding an option that is . by default. But this is just a workaround. The proper fix should be to configure whatever webserver you are running such that this works :/

@eric-wieser
Copy link
Contributor Author

2. But it does not perform a redirect to 404.html

This is a feature; performing a redirect would amount to serving up a 30x status code, which would not be the 404 status code that is meant to happen.

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

No branches or pull requests

3 participants