Add theme support for content of 401/403/404 responses

Is it still necessary?
With a Vanilla Drupal 8, templates suggestions follow the path so you can use html__system__40x or page__system__40x.

I'd much rather have a formal template defined than rely on magic path template naming. Path-based templating is a cludge anyway. :-) Not a priority but definitely open to someone working on this. I think this would also be 8.1 safe, probably?

Yeah, I think there would be no problem in making that an opt IN feature.

This is just a matter of updating \Drupal\system\Controller\Http4xxController AFAICT.

Yeah, you can also easily deal with it in a contrib module, if you like.

So yes, consider this approved to work on whenever. :-)

Attaching the patch from #2825147: No way to fully customise a 40x page by @chi.

I think this component is a better match.

Very strange, I tried to use the patch #11, but drupal continue to ignore file "page--404.html.twig"

@IRONFELIX, I just checked it against the latest 8.4.x code. The patch is still valid. Make sure you clear caches and template name is spelled correctly.

I use Drupal 8.3.1 and when I go to the wrong url, this line in code

$route_name = Drupal::routeMatch()->getRouteName();

returns value "entity.node.canonical" and next if doesn't work, of course.

entity.node.canonical means that valid route was found for the requested URL.
Can you check how this works on fresh Drupal installation?

@Chi, to tell the truth, I decided to select more easy way and set hook in my theme without touching any system files::

function mytheme_theme_suggestions_page_alter(array &$suggestions, array $variables) {
  $http_error_suggestions = [
     'system.401' => 'page__401',
     'system.403' => 'page__403',
     'system.404' => 'page__404',
  ];
  $node = \Drupal::request()->attributes->get('node');
  if ($node) {
     if ($node->id() == 49 ) {  // this is the node id linked to the 404 page
        $suggestions[] = $http_error_suggestions['system.404'];
     }
  }
}

@IRONFELIX, I suppose you have configured /node/49 path as default 404 page. That's why the patch did not work for you.

Works perfectly.

1. Install patch
2. Remove default error settings in: admin/config/system/site-information
3. Create a page--404.html.twig (and/or 401, 403)
4. Clear cache

Hello @IRONFELIX.
Just try the following code together with page--404.html.twig:

/**
 * Implements hook_theme_suggestions_page_alter() to set 40x template suggestions.
 *
 * @param array $suggestions
 * @param array $variables
 */
function mytheme_theme_suggestions_page_alter(array &$suggestions, array $variables) {
  $path_args = explode('/', trim(\Drupal::service('path.current')->getPath(), '/'));
  $suggestions = theme_get_suggestions($path_args, 'page');
  $http_error_suggestions = [
    'system.401' => 'page__401',
    'system.403' => 'page__403',
    'system.404' => 'page__404',
  ];

  $route_name = \Drupal::routeMatch()->getRouteName();
  if (isset($http_error_suggestions[$route_name])) {
    $suggestions[] = $http_error_suggestions[$route_name];
  }
}

#22 Works for me. Tested on several sites (Drupal 8.3.5).

Patch from #2363987-11: Add theme support for content of 401/403/404 responses works perfectly already on 2 client's projects. Both - Drupal 8.4.x.

We still need tests and a change record.

I had to slightly change #22 answer to use another hook (hook_theme_suggestions_HOOK_alter):

function MYTHEME_theme_suggestions_page_alter(array &$suggestions, array $variables){
  $path_args = explode('/', trim(\Drupal::service('path.current')->getPath(), '/'));

  $http_error_suggestions = [
    'system.401' => 'page__401',
    'system.403' => 'page__403',
    'system.404' => 'page__404',
  ];
  $route_name = \Drupal::routeMatch()->getRouteName();
  if (isset($http_error_suggestions[$route_name])) {
    $suggestions[] = $http_error_suggestions[$route_name];
  }
  
  return $suggestions;
}

Added a draft change record: https://www.drupal.org/node/2960810

$path_args variable is never used in #28. Here's updated code:

function MYTHEME_theme_suggestions_page_alter(array &$suggestions, array $variables){
  $http_error_suggestions = [
    'system.401' => 'page__401',
    'system.403' => 'page__403',
    'system.404' => 'page__404',
  ];
  $route_name = \Drupal::routeMatch()->getRouteName();
  if (isset($http_error_suggestions[$route_name])) {
    $suggestions[] = $http_error_suggestions[$route_name];
  }
  
  return $suggestions;
}

For those of us that want to keep the config items as an option, what do you all think of this as an option?

/**
 * Implements hook_theme_suggestions_HOOK_alter().
 */
function MYTHEME_theme_suggestions_page_alter(array &$suggestions, array $variables){

  // Define what we are handling.
  $http_error_suggestions = [
    '401' => 'page__4xx',
    '403' => 'page__4xx',
    '404' => 'page__4xx',
    'system.401' => 'page__4xx',
    'system.403' => 'page__4xx',
    'system.404' => 'page__4xx',
  ];

  // Determine if we have defined config vars.
  $site_403 = \Drupal::config('system.site')->get('page.403');
  $site_404 = \Drupal::config('system.site')->get('page.404');

  // If we do not have config vars, we can use route names.
  // Returns value "entity.node.canonical" when config vars are set.
  if (empty($site_403) && empty($site_404)) {
    $route_name = \Drupal::routeMatch()->getRouteName();
    if (isset($http_error_suggestions[$route_name])) {
      $suggestions[] = $http_error_suggestions[$route_name];
    }
  }
  else {
    $status = \Drupal::requestStack()->getCurrentRequest()->attributes->get('exception');
    if ($status) {
      $code = $status->getStatusCode();
      if (in_array($code, [401, 403, 404])) {
        $suggestions[] = $http_error_suggestions[$code];
      }
    }
  }

  return $suggestions;
}

#17 - gives me a solution

Added tests.

+++ b/core/tests/Drupal/KernelTests/Core/Theme/RegistryTest.php
@@ -193,6 +194,42 @@ public function testThemeSuggestions() {
+    $this->container->set('path.current', $path_current->reveal());
...
+    $suggestions = \Drupal::moduleHandler()->invokeAll('theme_suggestions_page', [[]]);

instead of $this->container better \Drupal::getContainer()

Thanks @andypost, updated the test. Not sure why the test in #36 is failing, but seems unrelated to this change.

Better using

$route_name = \Drupal::routeMatch()->getRouteName();

instead of

$route_name = Drupal::routeMatch()->getRouteName();

Also, another theme suggestion for `system.4xx` could be helpful as well.

Updated the patch, taking into account the feedback from #41.

Now ready!

RTBC +1

+++ b/core/tests/Drupal/KernelTests/Core/Theme/RegistryTest.php
@@ -193,6 +194,43 @@ public function testThemeSuggestions() {
+    $this->assertContains($suggestion, $suggestions);
+    $this->assertContains('page__4xx', $suggestions);

Could we add an assertion for the order of the suggestions as well? It would be good to ensure that the specific error codes override the generic one.

Check the suggestions

    $this->assertSame([
      'page__node',
      'page__node__%',
      'page__node__123',
      'page__4xx',
      $suggestion,
    ], $suggestions, 'Found expected suggestions for 401, 403 and 404 responses.');

1. Description for the @return value is missing.

/**
 * Data provider for test40xThemeSuggestions().
 *
 * @return array
 */
 public function provider40xThemeSuggestions() {

2. Two spaces.

diff --git a/core/tests/Drupal/KernelTests/Core/Theme/RegistryTest.php b/core/tests/Drupal/KernelTests/Core/Theme/RegistryTest.php
@@ -212,7 +212,7 @@ public function provider40xThemeSuggestions() {
    * @dataProvider provider40xThemeSuggestions
    */
   public function test40xThemeSuggestions($route, $suggestion) {
-    /** @var  Drupal\Core\Path\PathMatcherInterface $path_matcher */
+    /** @var Drupal\Core\Path\PathMatcherInterface $path_matcher */

reroll the patch with the comment from #49

+++ b/core/tests/Drupal/KernelTests/Core/Theme/RegistryTest.php
@@ -192,6 +193,49 @@ public function testThemeSuggestions() {
+    ], $suggestions, 'Found expected suggestions for 401, 403 and 404 responses.');

This is using a data provider, so only one code is getting tested per method run. We could use a variable in the assertion message maybe? Or just remove the assertion message.

Thanks @catch

Attached patch contains the following changes:

• modify the assertion message to a generic message

Changes according to #57.

Oh, wow! I'd swear this was done years ago, but clearly I was wrong!

Next complimentary target is #2231853: Rename maintenance-page template into page--maintenance + install-page into page--maintenance--install

#22 Works for me.