Fix HTML in site messages.

If we detect that a site message contains HTML then don't use the html_para filter to avoid adding extra <br> elements which mess up spacing.
mysociety · Feb 12, 2025 · e9952da · e9952da
1 parent 556b7f1
commit e9952da
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 5 deletions.
diff --git a/perllib/FixMyStreet/Template.pm b/perllib/FixMyStreet/Template.pm
@@ -156,6 +156,21 @@ sub html_paragraph_email_factory : FilterFactory('html_para_email') {
     }
 }
 
+=head2 html_paragraph_conditional
+
+Calls html_paragraph, but only if the text doesn't include
+a <p>, <ol>, or <ul>.
+
+=cut
+
+sub html_paragraph_conditional : Filter('html_para_conditional') {
+    my $text = shift;
+    if ($text !~ /<(p|ol|ul)[^>]*>/) {
+        $text = html_paragraph($text);
+    }
+    return $text;
+}
+
 sub sanitize {
     my ($text, $admin) = @_;
 

diff --git a/t/app/controller/admin/sitemessage.t b/t/app/controller/admin/sitemessage.t
@@ -153,6 +153,28 @@ FixMyStreet::override_config {
         $mech->get_ok('/');
         $mech->content_contains('This is an OOH message');
     };
+
+    subtest 'HTML vs non-HTML site messages' => sub {
+        # Test plain text message gets wrapped in paragraphs
+        $mech->get_ok('/admin/sitemessage');
+        my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/;
+        $mech->post_ok('/admin/sitemessage', {
+            site_message => "First line\n\nSecond line",
+            token => $csrf,
+        });
+        $mech->get_ok('/');
+        $mech->content_contains("<p>\nFirst line\n</p>\n\n<p>\nSecond line</p>");
+
+        # Test HTML message is left as-is
+        $mech->get_ok('/admin/sitemessage');
+        ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/;
+        $mech->post_ok('/admin/sitemessage', {
+            site_message => "<p>Test <strong>HTML</strong> message</p>\n\n<ul>\n<li>Item 1</li>\n<li>Item 2</li>\n</ul>",
+            token => $csrf,
+        });
+        $mech->get_ok('/');
+        $mech->content_contains("<p>Test <strong>HTML</strong> message</p>\r\n\r\n<ul>\r\n<li>Item 1</li>\r\n<li>Item 2</li>\r\n</ul>");
+    };
 };
 
 done_testing;
diff --git a/templates/web/base/front/site-message.html b/templates/web/base/front/site-message.html
@@ -1,7 +1,7 @@
 [% site_message = c.cobrand.site_message %]
 [% IF site_message && !rendered_site_message %]
 <div class="site-message">
-  [% site_message | html_para %]
+  [% site_message | html_para_conditional %]
 </div>
 [% END %]
 [% rendered_site_message = 1 %]
diff --git a/templates/web/base/report/new/form_after_heading.html b/templates/web/base/report/new/form_after_heading.html
@@ -1,7 +1,7 @@
 [% site_message = c.cobrand.site_message('reporting') %]
 [% IF site_message %]
 <div class="site-message__reporting">
-    [% site_message | safe | html_para %]
+    [% site_message | html_para_conditional %]
 </div>
 [% END %]
 
diff --git a/templates/web/base/waste/header.html b/templates/web/base/waste/header.html
@@ -20,7 +20,7 @@
 [% site_message = c.cobrand.site_message('waste') %]
 [% IF site_message %]
 <div class="site-message">
-  [% site_message | html_para %]
+  [% site_message | html_para_conditional %]
 </div>
 [% END %]
 [% IF site_message_upcoming_downtime %]

diff --git a/templates/web/bathnes/around/_postcode_form_post.html b/templates/web/bathnes/around/_postcode_form_post.html
@@ -1,6 +1,6 @@
 [% site_message = c.cobrand.site_message %]
 [% IF site_message %]
 <div class="front-main-other-issues-wrapper">
-  [% site_message | html_para %]
+  [% site_message | html_para_conditional %]
 </div>
 [% END %]
diff --git a/templates/web/peterborough/waste/header.html b/templates/web/peterborough/waste/header.html
@@ -17,6 +17,6 @@
 [% site_message = c.cobrand.site_message('waste') %]
 [% IF site_message %]
 <div class="site-message">
-  [% site_message | html_para %]
+  [% site_message | html_para_conditional %]
 </div>
 [% END %]