Allow custom 404.html on projects #5130

humitos · 2019-01-19T17:01:22Z

We had a discussion about this and we arrange on:

serve 404.html from the root of the version is currently visited if exists.
that 404.html page can be created manually or via the extesion sphinx-notfound-page.
we won't publish docs around this just yet to test it better

NOTE: considering the status of the extension, we don't want to make it a dependency of Read the Docs for all the builds yet because there are more work to do on it: render from a template, write tests and documentation.

Once the extension gets stable, we will include it in the build process and we will be generating better 404 pages for all the project without user action. Allowing the user to customize the default page if they prefer.

I tested this locally and it worked. I had to override some settings so Django calls handler_404:

PYTHON_MEDIA = True
DEBUG = False

after running runserver you can access to any not found page and see the custom one if the project has one.

Closes #353

humitos · 2019-01-19T17:01:58Z

readthedocs/core/views/__init__.py

+
+    if os.path.exists(fullpath):
+        log.debug('Serving custom 404.html page for project: %', project.slug)
+        r = HttpResponse(open(fullpath).read(), content_type='text/html')


I'm not sure if we want to serve this file from Django or we prefer to use the X-Accel-Redirect header here.

In fact, I think this is a bug (or a potential bug). I did the same at https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/core/views/serve.py#L269 but, what happen if this file is too big, for example?

I suppose that we should rely on NGINX to serve them.

I'd like to get some opinions here.

The main issue here is that it doesn't work locally. We should likely have it do Nginx when DEBUG=False, and serve locally when DEBUG=True.

Actually, it should perhaps respect PYTHON_MEDIA instead of DEBUG

Well, 404 handler is not called at all on DEBUG=True, so we will need DEBUG=False anyway to get at this point.

I agree that it should rely on PYTHON_MEDIA only.

I'm using readthedocs.core.views.serve._serve_file that does this for us ;)

humitos · 2019-01-19T17:02:25Z

readthedocs/core/views/__init__.py

+        filename = filename[1:]
+
+    if version.privacy_level == PRIVATE:
+        symlink = PrivateSymlink(project)


Do we need to do something special with private versions? I don't think so, but I'm not sure.

humitos · 2019-01-19T17:04:37Z

readthedocs/core/views/__init__.py

@@ -124,7 +129,36 @@ def server_error_404(request, exception=None, template_name='404.html'):  # pyli
            )
        else:
            return response
-    r = render(request, template_name)
+
+    project, path = project_and_path_from_request(request, request.get_full_path())


This function comes from readthedocs.redirects.utils but we are not using it here for any redirect. This may worth a refactor since this function is called here and also inside of get_redirect_response some lines above.

humitos · 2019-01-19T17:09:29Z

readthedocs/core/views/__init__.py

-    r = render(request, template_name)
+
+    project, path = project_and_path_from_request(request, request.get_full_path())
+    version_slug = project.get_default_version()


This fails when a 404 is generated from RTD itsefl because project is None, not a subdomain: http://localhost:8000/pagenotfound

I'm thinking on writing a special serve_error_404 for subdomains only and use it only on urls/subdomain.py. Any other idea?

Yea, we shouldn't replace the default 404 for RTD, just for subdomains.

docs/guides/custom-404-page.rst

ericholscher · 2019-01-21T21:45:18Z

docs/guides/custom-404-page.rst

+
+In case you want to follow the same style for the 404 page than your theme, you can either:
+
+1. manually copy the source of any already rendered pages or,


We shouldn't recommend this, but just point to the extension.

I removed this item and the whole "Manually creation" section.

ericholscher · 2019-01-29T22:02:02Z

@humitos we should get this put together and shipped.

humitos · 2019-02-04T12:39:21Z

We talked about installing the sphinx-notfound-page extension by default and always build a 404 page for projects we host, so they never see our own 404 Maze Found page.

humitos · 2019-02-04T13:23:05Z

This is an example of what is generated automatically by Read the Docs using sphinx-notfound-page.

It keeps the theme style, also maintains the flyout menu and all the toctree links in the left (we may want to remove the later since could not be the same among versions)

It has a CSS problem that I don't know what's happening and/or how to fix it yet.

Serve `404.html` from default version configured in the project if exists.

Co-Authored-By: humitos <humitos@gmail.com>

ericholscher · 2019-02-04T13:25:24Z

This is an example of what is generated automatically by Read the Docs using sphinx-notfound-page.

I think it's probably time to retire the ASCII art, and just use a normal 404 page. We can keep it for the main site, but it feels weird to inject into users docs. We should do something a bit more default and with less personality, since we don't know the style of the docs we're inserting into.

We use the sphinx-notfound-page extension to create a generic page for each project. By default we use the same Maze Found ASCII art, but the user can customize it by defining specific configs from the sphinx-notfound-page.

…age" This reverts commit 9ac46ef.

It uses the external sphinx-notfound-page extension for this.

humitos · 2019-02-06T16:09:51Z

I followed what we discussed and this PR is ready to review again and probably merge.

ericholscher

Looks good

ericholscher · 2019-02-06T16:35:10Z

docs/conf.py

+<p>Try using the search box or go to the homepage.</p>
+''',
+}
+


This is just for the future, right? We need to add the extension if we want it to do anything.

This is just of our docs to start testing it.

The extension is installed in our requirements.

I can remove it if you consider that it's best to start testing this in another dumb project, though.

humitos requested a review from a team January 19, 2019 17:01

humitos commented Jan 19, 2019

View reviewed changes

humitos added PR: work in progress Pull request is not ready for full review Needed: documentation Documentation is required Needed: tests Tests are required and removed Needed: documentation Documentation is required labels Jan 19, 2019

ericholscher reviewed Jan 21, 2019

View reviewed changes

humitos added the Feature New feature label Jan 31, 2019

humitos and others added 9 commits February 4, 2019 14:23

Allow custom 404.html on projects

55cc8e9

Serve `404.html` from default version configured in the project if exists.

Documentation guide for custom 404 page

a00c9f5

Update docs/guides/custom-404-page.rst

28b9a64

Co-Authored-By: humitos <humitos@gmail.com>

Update docs/guides/custom-404-page.rst

3791868

Co-Authored-By: humitos <humitos@gmail.com>

Update docs/guides/custom-404-page.rst

66ffbe9

Co-Authored-By: humitos <humitos@gmail.com>

Do not mention "Manually creation" of the 404 HTML

a33aee7

Add docstring to readthedocs.core.views.serve._serve_file

8f94b28

Use _serve_file to serve custom 404 page

c32422a

Serve custom 404 page only on subdomain and cname requests

689ec78

humitos force-pushed the humitos/allow-custom-404-pages branch from 48a1f55 to c25509c Compare February 4, 2019 13:23

humitos mentioned this pull request Feb 4, 2019

Improve 404 pages #2551

Closed

humitos force-pushed the humitos/allow-custom-404-pages branch 2 times, most recently from 9d819f6 to f7ce417 Compare February 4, 2019 15:14

humitos force-pushed the humitos/allow-custom-404-pages branch from f7ce417 to 9ac46ef Compare February 4, 2019 15:16

humitos removed the PR: work in progress Pull request is not ready for full review label Feb 4, 2019

Serve 404 page for current version first and fallback to default

fbfa68e

Test case for existing 404.html on project

d1d7e81

humitos removed the Needed: tests Tests are required label Feb 4, 2019

humitos added 5 commits February 5, 2019 09:07

Fix privacy level for test cases

ee2f595

Revert "Use sphinx-notfound-page by default to create a generic 404 p…

ca14616

…age" This reverts commit 9ac46ef.

Use getattr for checking subdomain and cname on request

71d5ec1

Build our docs with a custom 404 page

c09ad34

It uses the external sphinx-notfound-page extension for this.

Remove half backed documentation for custom 404 pages

a8f548a

humitos requested a review from a team February 6, 2019 16:10

humitos mentioned this pull request Feb 6, 2019

Show some useful links on 404 default page #5091

Closed

ericholscher approved these changes Feb 6, 2019

View reviewed changes

ericholscher merged commit 8f0c78d into master Feb 6, 2019

delete-merged-branch bot deleted the humitos/allow-custom-404-pages branch February 6, 2019 16:45

humitos mentioned this pull request Feb 6, 2019

Install sphinx-notfound-page for building 404.html custom page #5242

Merged

NicolasMassart mentioned this pull request Feb 19, 2019

404 page style PegaSysEng/pantheon#910

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow custom 404.html on projects #5130

Allow custom 404.html on projects #5130

humitos commented Jan 19, 2019 •

edited

Loading

humitos Jan 19, 2019

humitos Jan 19, 2019

ericholscher Jan 21, 2019

ericholscher Jan 21, 2019

humitos Jan 31, 2019

humitos Jan 31, 2019

humitos Jan 19, 2019 •

edited

Loading

humitos Jan 19, 2019

humitos Jan 19, 2019

humitos Jan 21, 2019 •

edited

Loading

ericholscher Jan 21, 2019

ericholscher Jan 21, 2019

humitos Jan 31, 2019

ericholscher commented Jan 29, 2019

humitos commented Feb 4, 2019

humitos commented Feb 4, 2019

ericholscher commented Feb 4, 2019

humitos commented Feb 6, 2019

ericholscher left a comment

ericholscher Feb 6, 2019

humitos Feb 6, 2019


		In case you want to follow the same style for the 404 page than your theme, you can either:

		1. manually copy the source of any already rendered pages or,

Allow custom 404.html on projects #5130

Allow custom 404.html on projects #5130

Conversation

humitos commented Jan 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos Jan 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericholscher commented Jan 29, 2019

humitos commented Feb 4, 2019

humitos commented Feb 4, 2019

ericholscher commented Feb 4, 2019

humitos commented Feb 6, 2019

ericholscher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos commented Jan 19, 2019 •

edited

Loading

humitos Jan 19, 2019 •

edited

Loading

humitos Jan 21, 2019 •

edited

Loading