fabiodalez-dev · fabiodalez-dev · Aug 1, 2026
diff --git a/app/Support/HtmlHelper.php b/app/Support/HtmlHelper.php
@@ -80,6 +80,35 @@ public static function sanitizeHtml(?string $html): string
         return self::htmlSanitizer()->sanitize($html);
     }
 
+    /**
+     * Render a rich-text field that may be EITHER editor HTML OR legacy plain
+     * text with newlines (imported records).
+     *
+     * Plain text gets nl2br so its line breaks survive. Content that is already
+     * block-structured HTML is left as-is: the editor (TinyMCE) separates its
+     * <p> paragraphs with a literal "\n", and running nl2br over that injects a
+     * stray <br> between every block, double-spacing the output (issue #313).
+     * The result is always sanitized.
+     *
+     * @param string|null $value Raw stored value
+     * @return string Sanitized HTML ready to echo
+     */
+    public static function richText(?string $value): string
+    {
+        if ($value === null || $value === '') {
+            return '';
+        }
+
+        // If the value already carries block/line HTML, its line breaks are
+        // markup — nl2br would turn the newlines *between* blocks into <br>.
+        $hasBlockHtml = preg_match(
+            '/<(?:p|br|div|ul|ol|li|h[1-6]|blockquote|pre|table|hr)\b/i',
+            $value
+        ) === 1;
+
+        return self::sanitizeHtml($hasBlockHtml ? $value : nl2br($value, false));
+    }
+
     /**
      * Return a public absolute HTTP(S) URL, or an empty string when unsafe.
      *

diff --git a/app/Views/frontend/book-detail.php b/app/Views/frontend/book-detail.php
@@ -2026,7 +2026,7 @@ class="book-cover-large img-fluid"
                                         : \App\Support\MediaLabels::formatTracklist($musicDescription) ?>
                                 </div>
                             <?php else: ?>
-                                <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::sanitizeHtml(nl2br($book['descrizione'], false)) ?></div>
+                                <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::richText($book['descrizione']) ?></div>
                             <?php endif; ?>
                         <?php else: ?>
                             <p class="text-gray-500"><?= __("Nessuna descrizione disponibile per questo libro.") ?></p>
@@ -2263,7 +2263,7 @@ class="status-badge bg-gray-100 text-gray-900 border px-3 py-2 no-underline keyw
                                     <?php elseif (in_array($fieldName, ['value'])): ?>
                                         € <?= number_format((float)$field['value'], 2) ?>
                                     <?php elseif (in_array($fieldName, ['review', 'comment'])): ?>
-                                        <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::sanitizeHtml(nl2br($field['value'], false)) ?></div>
+                                        <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::richText($field['value']) ?></div>
                                     <?php else: ?>
                                         <?= htmlspecialchars($field['value'], ENT_QUOTES, 'UTF-8') ?>
                                     <?php endif; ?>
@@ -2297,7 +2297,7 @@ class="status-badge bg-gray-100 text-gray-900 border px-3 py-2 no-underline keyw
                                     <?php elseif (in_array($fieldName, ['value'])): ?>
                                         € <?= number_format((float)$field['value'], 2) ?>
                                     <?php elseif (in_array($fieldName, ['review', 'comment'])): ?>
-                                        <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::sanitizeHtml(nl2br($field['value'], false)) ?></div>
+                                        <div class="prose prose-sm max-w-none"><?= \App\Support\HtmlHelper::richText($field['value']) ?></div>
                                     <?php else: ?>
                                         <?= htmlspecialchars($field['value'], ENT_QUOTES, 'UTF-8') ?>
                                     <?php endif; ?>

diff --git a/app/Views/libri/scheda_libro.php b/app/Views/libri/scheda_libro.php
@@ -718,7 +718,7 @@ class="inline-flex items-center gap-1 px-2 py-1 text-xs font-medium rounded-full
               ?>
               <?= \App\Support\MediaLabels::formatTracklist($descText) ?>
             <?php else: ?>
-              <?php echo App\Support\HtmlHelper::sanitizeHtml(nl2br($libro['descrizione'], false)); ?>
+              <?php echo App\Support\HtmlHelper::richText($libro['descrizione']); ?>
             <?php endif; ?>
           </div>
         </div>
@@ -805,7 +805,7 @@ class="inline-flex items-center gap-1 px-2 py-1 text-xs font-medium rounded-full
                       echo ' <span class="text-gray-600">(' . $rating . '/5)</span>';
                   } elseif ($fieldName === 'review' || $fieldName === 'comment') {
                       // Multi-line text
-                      echo '<div class="prose prose-sm max-w-none">' . App\Support\HtmlHelper::sanitizeHtml(nl2br($field['value'], false)) . '</div>';
+                      echo '<div class="prose prose-sm max-w-none">' . App\Support\HtmlHelper::richText($field['value']) . '</div>';
                   } elseif (in_array($fieldName, ['entry_date', 'date_started', 'date_read', 'lending_start', 'lending_end'])) {
                       // Date formatting
                       echo App\Support\HtmlHelper::e(format_date($field['value'], false, '/'));

diff --git a/tests/richtext-nl2br-313.unit.php b/tests/richtext-nl2br-313.unit.php
@@ -0,0 +1,71 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * Regression test for issue #313 — the OPAC injected <br> between the <p>
+ * paragraphs of a book description.
+ *
+ * Root cause: the book description (and custom rich-text fields) were rendered
+ * with sanitizeHtml(nl2br($value, false)). Editor HTML separates its <p> blocks
+ * with a literal "\n", so nl2br turned every inter-block newline into a stray
+ * <br>, double-spacing the output. HtmlHelper::richText() applies nl2br only to
+ * plain-text values and leaves block HTML untouched.
+ *
+ * Run:  php tests/richtext-nl2br-313.unit.php   (exit 0 iff all pass)
+ */
+
+$root = dirname(__DIR__);
+require $root . '/vendor/autoload.php';
+
+use App\Support\HtmlHelper;
+
+$pass = 0;
+$fail = 0;
+$check = static function (bool $ok, string $label) use (&$pass, &$fail): void {
+    if ($ok) { $pass++; echo "  OK  {$label}\n"; }
+    else     { $fail++; echo "  FAIL {$label}\n"; }
+};
+
+// ── 1. The bug: editor HTML must NOT gain <br> between paragraphs ────────────
+$html = "<p>Para uno.</p>\n<p>Para due.</p>\n<p>Para tre.</p>";
+$out = HtmlHelper::richText($html);
+$check(!str_contains($out, '<br'), "01 HTML paragraphs render without an injected <br> (issue #313)");
+$check(substr_count($out, '<p>') === 3, "02 all three paragraphs are preserved");
+
+// ── 2. Plain text still gets its line breaks ────────────────────────────────
+$plain = "Prima riga\nSeconda riga\nTerza riga";
+$out = HtmlHelper::richText($plain);
+$check(substr_count($out, '<br') === 2, "03 plain text keeps its newlines as <br>");
+
+// A single-block value that only uses <br> is already HTML → left as-is (no
+// doubling): the two <br> stay two, they are not turned into four.
+$brHtml = "Riga uno<br>Riga due";
+$out = HtmlHelper::richText($brHtml);
+$check(substr_count($out, '<br') === 1, "04 a <br>-only value is treated as HTML, not re-broken");
+
+// Other block markers are detected too (div / ul / heading).
+foreach (['<div>x</div>', '<ul><li>x</li></ul>', '<h2>x</h2>'] as $i => $block) {
+    $withNl = $block . "\ntail";
+    $out = HtmlHelper::richText($withNl);
+    // The trailing "\ntail" newline must not become a <br> when the value is HTML.
+    $check(!str_contains($out, '<br'), sprintf("0%d block HTML (%s) suppresses nl2br", 5 + $i, strtok($block, '>') . '>'));
+}
+
+// ── 3. Sanitisation is still applied ────────────────────────────────────────
+$out = HtmlHelper::richText('<p>ok</p><script>alert(1)</script>');
+$check(!str_contains($out, '<script'), "08 script tags are still stripped (sanitised)");
+
+$out = HtmlHelper::richText("plain <script>alert(1)</script> text");
+$check(!str_contains($out, '<script'), "09 script in plain-text path is stripped too");
+
+// ── 4. Empty / null ─────────────────────────────────────────────────────────
+$check(HtmlHelper::richText(null) === '', "10 null => empty string");
+$check(HtmlHelper::richText('') === '', "11 empty => empty string");
+
+// ── 5. Cyrillic content (the reporter's data) survives ──────────────────────
+$cyr = "<p>Америка&hellip;</p>\n<p>Однако&hellip;</p>";
+$out = HtmlHelper::richText($cyr);
+$check(str_contains($out, 'Америка') && !str_contains($out, '<br'), "12 Cyrillic paragraphs render clean (reporter's case)");
+
+echo "\n{$pass} PASS, {$fail} FAIL\n";
+exit($fail === 0 ? 0 : 1);