canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
Other
13 stars 35 forks source link

404 page / notfound extension #192

Closed ru-fu closed 5 months ago

ru-fu commented 6 months ago

https://github.com/canonical/sphinx-docs-starter-pack/pull/180 removed the notfound extension in favor of a single 404 page.

I now tested this, and (as much as I like the bird :wink: ) it's complicated ... First some notes about the setup:

I've tested the following URLs (the relevant ones are the ones in bold, see above):

  1. <baseURL>/nonexistingpage
  2. <baseURL>/nonexistingpage/
  3. <baseURL>/existingpage/nonexistingpage
  4. <baseURL>/existingpage/nonexistingpage/

The result

For *.readthedocs-hosted.com:

Test Current (no extension) with versions Current (no extension) without versions Before (notfound extension) with versions Before (notfound extension) without versions
1 :no_entry: :ok: :ok: :no_entry:
2 :ok: :ok: :ok: :no_entry:
3 :ok: :ok: :ok: :no_entry:
4 :no_entry: :no_entry: :ok: :no_entry:

For documentation.ubuntu.com/*:

Test Current (no extension) with versions Current (no extension) without versions Before (notfound extension) with versions Before (notfound extension) without versions
1 :no_entry: :no_entry: :ok: :no_entry:
2 :ok: :ok: :ok: :no_entry:
3 :ok: :ok: :ok: :no_entry:
4 :no_entry: :no_entry: :ok: :no_entry:

So basically, the current solution works in some cases and not in others. There also isn't really anything we can do about the failures (especially case 4 which is relevant) - we would need to copy the 404 page into all subfolders for that. :worried: The old solution with the extension works for en/latest/ style URLs but not for others. However, there we can tweak the behaviour - we already did that for the different URLs (see slug and notfound_urls_prefix).

So my suggestion is to revert to using the extension, and add some logic to notfound_urls_prefix to make it work without versions. We can probably also add a custom template for the page so we can keep the bird. :wink:

Opinions, @dviererbe and @pmatulis ?

pmatulis commented 6 months ago

@ru-fu Thanks for such a thorough test. I agree that we should revert if we cannot fix the brokenness without the extension (but can fix it with the extension).