Problem/Motivation

When adding an entry to the 4040 ignore field, the code adds an extra white line between lines.

Proposed resolution

Avoid adding that extra line.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 interdiff-3143879-2-6.txt2.22 KBjohnchque
#6 3143879-extra-whiteline-6.patch2.99 KBjohnchque
#6 3143879-extra-whiteline-6-TEST-ONLY.patch2.37 KBjohnchque
#2 3143879-extra-whiteline-2.patch636 bytesjohnchque

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Active » Needs review
StatusFileSize
new 3143879-extra-whiteline-2.patch636 bytes

This should work. Gonna check if it breaks any test.

shreyal999’s picture

shreyal999 commented

Picking this up for review.

shreyal999’s picture

shreyal999 commented

Hey yongt9412, can you help me with the module as it says.
redirect_404 was not found. [warning]
No release history available for redirect_404 8.x .

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work
Issue tags: +Needs tests 
+++ b/modules/redirect_404/redirect_404.module
@@ -46,7 +46,7 @@ function redirect_404_form_redirect_settings_form_alter(&$form, FormStateInterfa
   // Add a new path to be ignored, if there is an ignore argument in the query.
   if ($path_to_ignore = \Drupal::request()->query->get('ignore')) {
-    $ignored_pages .= "\n" . $path_to_ignore;
+    $ignored_pages .= $path_to_ignore;
   }

can we test this somehow? For some reason, we thought this was necessary.

Maybe save the form and then assert what exactly has been stored?

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new 3143879-extra-whiteline-6-TEST-ONLY.patch2.37 KB
new 3143879-extra-whiteline-6.patch2.99 KB
new interdiff-3143879-2-6.txt2.22 KB

Hmmm I guess that in tests we were missing this line in the end. This should not break other tests.

We also were asserting that the container had that text but not asserting the whitelines.

The last submitted patch, 6: 3143879-extra-whiteline-6-TEST-ONLY.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed
Issue tags: -Needs tests

Committed.

  • Berdir committed e5201ca on 8.x-1.x authored by yongt9412 
    Issue #3143879 by yongt9412: Remove extra white line when adding...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.