I've added the possibility to translate the html error page, replacing a token with the current language prefix.
It only work for websites using the language prefix.
Feel free to improve this with a query to get the domains, and handle the language by domain.

How it works ?

$conf['fast_404_HTML_error_page'] = './404/index_##LANG##.html';

The ##LANG## token will be replaced by the current language prefix.

You can have index_.html, index_en.html, index_it.html, etc.

CommentFileSizeAuthor
#40 interdiff-38.txt2.41 KBseanB
#39 2370323-38-10.0.patch11.9 KBseanB
#38 2370323-38.patch10.89 KBseanB
#36 interdiff_31-36.txt723 bytesmrinalini9
#36 2370323_36.patch11.19 KBmrinalini9
#31 interdiff_27_31.txt2.02 KBlucasbaralm
#31 2370323_31.patch11.21 KBlucasbaralm
#27 interdiff_12_27.txt3.38 KBjoshua1234511
#27 2370323_27.patch11.24 KBjoshua1234511
#24 interdiff_12-23.txt4.94 KBelber
#24 2370323-23.patch7.87 KBelber
#20 interdiff_17-20.txt1.11 KBelber
#20 2370323-19.patch7.85 KBelber
#17 interdiff_12-17.txt4.58 KBsrilakshmier
#17 2370323-17.patch7.85 KBsrilakshmier
#12 2370323-12.interdiff.txt4.14 KBYahyaAlHamad
#12 2370323-12.patch11.23 KBYahyaAlHamad
#10 2370323-10.patch6.72 KBYahyaAlHamad
#6 fast_404-2370323-6.patch8.97 KBgapple
#4 fast_404-2370323-4.patch8.9 KBgapple
#1 fast_404_translatable_html_error_page_2370323.patch2.18 KBcutesquirrel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cutesquirrel’s picture

cutesquirrel CreditAttribution: cutesquirrel commented
Issue summary: View changes
FileSize
fast_404_translatable_html_error_page_2370323.patch2.18 KB
adammalone’s picture

adammalone
Primary language English
Location Sydney
CreditAttribution: adammalone commented
Status: Patch (to be ported) » Needs work

The only way I could see this working with early stage bootstrap is if the path was parsed and determined to contain a language or if languages were found from the database (We already query the locale table). My concern about this is how we would know which language was desired, I'm not confident about replacing tokens.

This patch also introduces whitespace and shouldn't introduce another global.

alexander.sibert’s picture

alexander.sibert CreditAttribution: alexander.sibert commented

Great module and i'm also interest on this feature. I use on a current project i18n with lang codes.

gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple commented
Status: Needs work » Needs review
FileSize
fast_404-2370323-4.patch8.9 KB

Here's a patch that checks if the requested path is prefixed with a valid language.

Status: Needs review » Needs work

The last submitted patch, 4: fast_404-2370323-4.patch, failed testing.

gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple commented
Status: Needs work » Needs review
FileSize
fast_404-2370323-6.patch8.97 KB

Here's an updated patch for latest dev

gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple commented

I ran into a weird issue on Acquia hosting where the Database API functions were available when fast_404_path_check() was called in settings.php.

This caused problems with my patches, because the variables are not bootstrapped yet, and drupal_multilingual() then always returns false.

I fixed this by using the check for the locale module's status, as was originally used in fast_404_validate_path_drupal(), instead of drupal_multilingual().

eddie_c’s picture

eddie_c CreditAttribution: eddie_c at Oxfam International commented
Version: 7.x-1.x-dev » 8.x-1.x-dev

It would be great to have this feature in the 8.x-1.x branch. It may be simpler to achieve in the D8 version by parsing the language prefix from the path in Fast404->response() and then using that to load a language appropriate static HTML file in a way similar to the one suggested here by @cutesquirrel

Perhaps this should be worked on for Drupal 8 first and then backported to D7 in order to make sure this feature isn't abandoned going forward?

Optimistically bumping the version of this issue up to 8.x!

gg4’s picture

gg4 CreditAttribution: gg4 commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
YahyaAlHamad’s picture

YahyaAlHamad
Location Palestine
CreditAttribution: YahyaAlHamad commented
FileSize
2370323-10.patch6.72 KB

Added the ability to translate in version 2, I'm not sure if this is the best way. Read the example.settings.fast404.php for more info.

Status: Needs review » Needs work

The last submitted patch, 10: 2370323-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

YahyaAlHamad’s picture

YahyaAlHamad
Location Palestine
CreditAttribution: YahyaAlHamad commented
FileSize
2370323-12.patch11.23 KB
2370323-12.interdiff.txt4.14 KB

Fixed the test, also, added a new test, to test 'fast404_HTML_error_page' functionality.

YahyaAlHamad’s picture

YahyaAlHamad
Location Palestine
CreditAttribution: YahyaAlHamad commented
Status: Needs work » Needs review

I used the LanguageManager service to determine the current language, I fear that I have missed the whole point of the module.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Needs work

Thanks for the issue, patches, and reviews!

I took a quick look and have some minor feedback, mostly nitpicks. Moving back to needs work for at least fixing the typos.

  1. +++ b/src/EventSubscriber/Fast404EventSubscriber.php
@@ -25,14 +26,24 @@ class Fast404EventSubscriber implements EventSubscriberInterface {
+   * The language mananger

    * Typo: "mananger" => "manager".
    * Nitpick: Add period.

  2. +++ b/src/Fast404.php
@@ -40,14 +40,24 @@ class Fast404 {
+   * The langcode, this will help us to load the translated 404 page if it exists.

    Nitpick: This goes beyond 80 characters. Perhaps reword slightly, e.g.

    The langcode to use for the translated 404 page if it exists.

  3. +++ b/src/Fast404.php
@@ -198,10 +208,20 @@ class Fast404 {
+        // If it is an array, get path depending on the langcode, if langcode is NULL, default to the first value.

    * Nitpick: This goes beyond 80 characters.
    * Nitpick: These should be 2 separate sentences or be separated by a ";" instead of a comma.

  4. +++ b/src/Fast404.php
@@ -198,10 +208,20 @@ class Fast404 {
+      } else {
+        $custom_404_path = $custom_404;

    Nitpick: Move else to own line, e.g.

    }
else {

  5. +++ b/src/Fast404.php
@@ -198,10 +208,20 @@ class Fast404 {
+      if(file_exists($custom_404_path)) {

    Nitpick: Add space after if.

  6. +++ b/tests/html/en.html
@@ -0,0 +1,14 @@
+	<head>
+		<meta charset="UTF-8">
+		<meta http-equiv="X-UA-Compatible" content="IE=edge">
+		<meta name="viewport" content="width=device-width, initial-scale=1.0">
+		<title>Not Found</title>
+	</head>
+	<body>
+		<h1>
+			Custom Page not found
+		</h1>
+	</body>

+++ b/tests/html/fr.html
@@ -0,0 +1,14 @@
+	<head>
+		<meta charset="UTF-8">
+		<meta http-equiv="X-UA-Compatible" content="IE=edge">
+		<meta name="viewport" content="width=device-width, initial-scale=1.0">
+		<title>Pas trouvé</title>
+	</head>
+	<body>
+		<h1>
+			Page personnalisée introuvable
+		</h1>
+	</body>

    * Nitpick: Should these use spaces instead of tabs?
    * Nitpick: Change "Custom Page not found" to "Custom Page Not Found".

  7. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    // Let fast404 subscribe to NotFoundHttpException

    Nitpick: Add period.

  8. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    // Try to access a non-existant page.

    Typo: "existant" => "existent".

  9. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    $this->assertSession()->pageTextContains('Custom Page not found');

    Nitpick: Change "Custom Page not found" to "Custom Page Not Found".

  10. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    // Try to access a non-existant page.

    Typo: "existant" => "existent".

  11. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    $this->assertSession()->pageTextContains('Custom Page not found');

    Nitpick: Change "Custom Page not found" to "Custom Page Not Found".

  12. +++ b/tests/src/Functional/Fast404HtmlPageTest.php
@@ -0,0 +1,80 @@
+    // Try to access a non-existant page.

    Typo: "existant" => "existent".

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Issue tags: +Needs issue summary update

Looks like the approach changed, so the issue summary will need updating.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Issue tags: +Novice

Adding novice tag as changes to patch are simple.

srilakshmier’s picture

srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commented
Status: Needs work » Needs review
FileSize
2370323-17.patch7.85 KB
interdiff_12-17.txt4.58 KB

Uploading the patch based on the feedback in #14.
Please review.

Thank you

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Needs work

Thanks for the update. I reviewed the interdiff. Not sure why the tests were removed so moving back to needs work for adding those back and I noticed a couple minor things as well.

  1. +++ b/src/Fast404.php
@@ -213,13 +213,14 @@
-        // If it is an array, get path depending on the langcode, if langcode is NULL, default to the first value.
+        // For array, get path depending on the langcode; if langcode is NULL, default to the first value.

    Update to wrap comment at <80 characters. 

  2. +++ b/src/Fast404.php
@@ -213,13 +213,14 @@
+      } ¶
+      else {

    Extra space after the }. 

  3. +++ b/src/Fast404.php
@@ -213,13 +213,14 @@
--- b/tests/html/en.html

--- b/tests/html/en.html
+++ /dev/null

    Shouldn't be removed. 

  4. +++ /dev/null
@@ -1,14 +0,0 @@
--- b/tests/html/fr.html

--- b/tests/html/fr.html
+++ /dev/null

    Shouldn't be removed. 

  5. +++ /dev/null
@@ -1,14 +0,0 @@
--- b/tests/src/Functional/Fast404HtmlPageTest.php

--- b/tests/src/Functional/Fast404HtmlPageTest.php
+++ /dev/null

    Shouldn't be removed.

  6. Most of the feedback from #14 was not addressed.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Assigned: Unassigned » elber
elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Assigned: elber » Unassigned
Status: Needs work » Needs review
FileSize
2370323-19.patch7.85 KB
interdiff_17-20.txt1.11 KB

My patch has the changes mentioned in the comment #18, and I didn't exclude the HTML files.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Needs work

Thank you for your attempt at helping @elber but this is still using the bad patch from #17 which is missing tests. This is noted in #18 but I guess I didn't make that obvious enough so...

Whoever works on this next:

1. PLEASE USE PATCH FROM #12.

2. Then look at the feedback from #14 and #18.

3. Do an interdiff between #12 and your updated patch. (The patches in #17 and #20 broke things like removing tests.)

Thanks.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Assigned: Unassigned » elber
elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented

I'm still working on this!

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Assigned: elber » Unassigned
Status: Needs work » Needs review
FileSize
2370323-23.patch7.87 KB
interdiff_12-23.txt4.94 KB
Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Status: Needs review » Needs work

Looked at the interdiff and see the test files being removed. Also, make sure there are no extra spaces at the end of lines.

Back to needs work. 

  1. +++ b/src/Fast404.php
@@ -213,13 +213,15 @@
--- b/tests/html/en.html

--- b/tests/html/en.html
+++ /dev/null

    Why are you removing test files? 

  2. +++ /dev/null
@@ -1,14 +0,0 @@
--- b/tests/html/fr.html

--- b/tests/html/fr.html
+++ /dev/null

    Why are you removing test files? 

  3. +++ /dev/null
@@ -1,14 +0,0 @@
--- b/tests/src/Functional/Fast404HtmlPageTest.php

--- b/tests/src/Functional/Fast404HtmlPageTest.php
+++ /dev/null

    Why are you removing test files?

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Assigned: Unassigned » elber
joshua1234511’s picture

joshua1234511
Primary language English
Location Goa
CreditAttribution: joshua1234511 as a volunteer and at Axelerant for Salsa Digital commented
Status: Needs work » Needs review
FileSize
2370323_27.patch11.24 KB
interdiff_12_27.txt3.38 KB

Updated the path from #12
Worked on feedback from #14
#13 no specific feedback to be worked on just as #14 was not addressed.

Additional Addresed
FILE: /fast_404/tests/src/Functional/Fast404HtmlPageTest.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------
79 | ERROR | [x] Expected 1 blank line after function; 0 found
80 | ERROR | [x] The closing brace for the class must have an empty line before it

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Issue tags: -Novice +Needs steps to reproduce, +Needs manual testing

Thanks @joshua1234511!

We should add some explicit testing steps so people can try this.

This still needs code review too.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Assigned: elber » Unassigned
Status: Needs review » Needs work

Thanks for updating the patch and cleaning up the code formatting issues. I reviewed again and there are a couple things:

  1. +++ b/src/Fast404.php
@@ -198,10 +208,22 @@ class Fast404 {
+        // If it is an array, get path depending on the langcode;
+        // if langcode is NULL, default to the first value.

    Comments shouldn't read like code, so this should be reworded to something like:

    For arrays, use the path associated with the langcode or the first path if the langcode is not set. 

  2. +++ b/tests/html/en.html
@@ -0,0 +1,14 @@
+	<head>
+		<meta charset="UTF-8">
+		<meta http-equiv="X-UA-Compatible" content="IE=edge">
+		<meta name="viewport" content="width=device-width, initial-scale=1.0">
+		<title>Not Found</title>
+	</head>
+	<body>
+		<h1>
+			Custom Page Not Found
+		</h1>
+	</body>

    1. The indentation here does not follow indentation seen in other html files in core, e.g. core/modules/color/preview.html.

    2. Nitpick: Should Custom Page Not Found be wrapped in 

    <h1></h1>

    on one line? e.g.

    <h1>Custom Page Not Found</h1>

  3. +++ b/tests/html/fr.html
@@ -0,0 +1,14 @@
+	<head>
+		<meta charset="UTF-8">
+		<meta http-equiv="X-UA-Compatible" content="IE=edge">
+		<meta name="viewport" content="width=device-width, initial-scale=1.0">
+		<title>Pas trouvé</title>
+	</head>
+	<body>
+		<h1>
+			Page personnalisée introuvable
+		</h1>
+	</body>

    Same as above.

lucasbaralm’s picture

lucasbaralm
He/Him
CreditAttribution: lucasbaralm at CI&T commented
Assigned: Unassigned » lucasbaralm
lucasbaralm’s picture

lucasbaralm
He/Him
CreditAttribution: lucasbaralm at CI&T commented
Status: Needs work » Needs review
FileSize
2370323_31.patch11.21 KB
interdiff_27_31.txt2.02 KB

Fixing standards and comments mentioned in #29.

lucasbaralm’s picture

lucasbaralm
He/Him
CreditAttribution: lucasbaralm at CI&T commented
Assigned: lucasbaralm » Unassigned
diegors’s picture

diegors
He/Him
Primary language Portuguese, Brazil
Location Campinas - Brazil
CreditAttribution: diegors at CI&T commented
Assigned: Unassigned » diegors
diegors’s picture

diegors
He/Him
Primary language Portuguese, Brazil
Location Campinas - Brazil
CreditAttribution: diegors at CI&T commented
Assigned: diegors » Unassigned
Status: Needs review » Reviewed & tested by the community

@lucasbaralm has fixed standards mentioned in comment #29.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte for Salsa Digital commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Novice

Some minor formatting issues remain:

+++ b/src/Fast404.php
@@ -213,8 +213,9 @@
+        // For arrays, use the path associated with the langcode or the ¶
+        //first path if the langcode is not set.
+        ¶

There's some whitespace and wrapping issues here.

1. "first" can move a line up
2. need space after "//"
3. do not have extraneous spaces at end of lines
4. empty line not needed

mrinalini9’s picture

mrinalini9 CreditAttribution: mrinalini9 at Material for Drupal India Association commented
Status: Needs work » Needs review
FileSize
2370323_36.patch11.19 KB
interdiff_31-36.txt723 bytes

Updated patch #31 by addressing #35, please review it.

Thanks!

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Version: 8.x-2.x-dev » 8.x-3.x-dev
seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented
FileSize
2370323-38.patch10.89 KB

Reroll for 3.x.

seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented
FileSize
2370323-38-10.0.patch11.9 KB

Fixed coding standards and made the patch compatible with 10.x (the method signature for BrowserTestBase::setUp() changed).

seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented
FileSize
interdiff-38.txt2.41 KB

Yay! Tests are green. Here is the interdiff for the latest 9.x and 10.x patches of #38 / #39.

YahyaAlHamad’s picture

YahyaAlHamad
Location Palestine
CreditAttribution: YahyaAlHamad at Sprintive commented
akashpj’s picture

akashpj CreditAttribution: akashpj as a volunteer and at Cognizant Technology Solutions for Drupal India Association commented

Tested #39 patch in drupal 9.5.x and 10.1.x
patch applied successfully and working as expected
Verified the changes suggested in #35 - Changes are addressed.