From 16c3785b52cf7fc50ead4ba01f63a4d40f28d80a Mon Sep 17 00:00:00 2001
From: Nazar Gargol <nazargargol@gmail.com>
Date: Mon, 26 Aug 2019 17:01:20 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20lack=20of=20space=20in?=
 =?UTF-8?q?=20excerpt=20generated=20from=20paragraphs?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes #10531

- Adds space when encountering closing </p> and <br> tags
- The mobiledoc-to-html conversion produces these tags in this exact
syntax, so there is no need to account for more cases like additional spaces or alternative syntax like  <br />
- Added test cases that cover spacing use-casei
- Changed some existing tests to contain more close-to-real-world markup
- The downside of this approach is generating multiple spaces in case there are empty paragraphs in the markup. The same concern is true for current "new line" substitution:
> excerpt.replace(/(\r\n|\n|\r)+/gm, ' ')
but it never has been a concern as in real world when multiple spaces are used inside of the `<p>` tag they are rendered as single space.
---
 core/frontend/meta/excerpt.js            |  4 ++
 core/test/unit/data/meta/excerpt_spec.js |  8 ++--
 core/test/unit/helpers/excerpt_spec.js   | 54 +++++++++++++++++++++++-
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/core/frontend/meta/excerpt.js b/core/frontend/meta/excerpt.js
index eb4f6c2043..f194ed3e85 100644
--- a/core/frontend/meta/excerpt.js
+++ b/core/frontend/meta/excerpt.js
@@ -5,6 +5,10 @@ function getExcerpt(html, truncateOptions) {
     // Strip inline and bottom footnotes
     var excerpt = html.replace(/<a href="#fn.*?rel="footnote">.*?<\/a>/gi, '');
     excerpt = excerpt.replace(/<div class="footnotes"><ol>.*?<\/ol><\/div>/, '');
+
+    // Make sure to have space between paragraphs and new lines
+    excerpt = excerpt.replace(/(<\/p>|<br>)/gi, ' ');
+
     // Strip other html
     excerpt = excerpt.replace(/<\/?[^>]+>/gi, '');
     excerpt = excerpt.replace(/(\r\n|\n|\r)+/gm, ' ');
diff --git a/core/test/unit/data/meta/excerpt_spec.js b/core/test/unit/data/meta/excerpt_spec.js
index 088865b1ed..cafe4e2d08 100644
--- a/core/test/unit/data/meta/excerpt_spec.js
+++ b/core/test/unit/data/meta/excerpt_spec.js
@@ -5,9 +5,9 @@ describe('getExcerpt', function () {
     it('should return html excerpt with no html', function () {
         var html = '<p>There are <br />10<br> types<br/> of people in <img src="a">the world:' +
                 '<img src=b alt="c"> those who <img src="@" onclick="javascript:alert(\'hello\');">' +
-                'understand trinary</p>, those who don\'t <div style="" class=~/\'-,._?!|#>and' +
+                'understand trinary,</p> those who don\'t <div style="" class=~/\'-,._?!|#>and' +
                 '< test > those<<< test >>> who mistake it &lt;for&gt; binary.',
-            expected = 'There are 10 types of people in the world: those who understand trinary, those who ' +
+            expected = 'There are 10  types of people in the world: those who understand trinary,  those who ' +
                 'don\'t and those>> who mistake it &lt;for&gt; binary.';
 
         getExcerpt(html, {}).should.equal(expected);
@@ -61,9 +61,9 @@ describe('getExcerpt', function () {
         function () {
             var html = '<p>There are <br />10<br> types<br/> of people in <img src="a">the world:' +
                     '<img src=b alt="c"> those who <img src="@" onclick="javascript:alert(\'hello\');">' +
-                    'understand trinary</p>, those who don\'t <div style="" class=~/\'-,._?!|#>and' +
+                    'understand trinary,</p> those who don\'t <div style="" class=~/\'-,._?!|#>and' +
                     '< test > those<<< test >>> who mistake it &lt;for&gt; binary.',
-                expected = 'There are 10 types of people in the world: those who understand trinary, those who ' +
+                expected = 'There are 10  types of people in the world: those who understand trinary,  those who ' +
                     'don\'t and those>> who mistake it &lt;for&gt; binary.';
 
             getExcerpt(html).should.equal(expected);
diff --git a/core/test/unit/helpers/excerpt_spec.js b/core/test/unit/helpers/excerpt_spec.js
index ef3eddbb11..6e9f47968a 100644
--- a/core/test/unit/helpers/excerpt_spec.js
+++ b/core/test/unit/helpers/excerpt_spec.js
@@ -29,9 +29,9 @@ describe('{{excerpt}} Helper', function () {
     it('does not output HTML', function () {
         var html = '<p>There are <br />10<br> types<br/> of people in <img src="a">the world:' +
                 '<img src=b alt="c"> those who <img src="@" onclick="javascript:alert(\'hello\');">' +
-                'understand trinary</p>, those who don\'t <div style="" class=~/\'-,._?!|#>and' +
+                'understand trinary,</p> those who don\'t <div style="" class=~/\'-,._?!|#>and' +
                 '< test > those<<< test >>> who mistake it &lt;for&gt; binary.',
-            expected = 'There are 10 types of people in the world: those who understand trinary, those who ' +
+            expected = 'There are 10  types of people in the world: those who understand trinary,  those who ' +
                 'don\'t and those>> who mistake it &lt;for&gt; binary.',
             rendered = helpers.excerpt.call({
                 html: html,
@@ -182,4 +182,54 @@ describe('{{excerpt}} Helper', function () {
         should.exist(rendered);
         rendered.string.should.equal(expected);
     });
+
+    it('puts additional space after closing paragraph', function () {
+        var html = '<p>Testing.</p><p>Space before this text.</p><p>And this as well!</p>',
+            expected = 'Testing. Space before this text. And this as well!',
+            rendered = (
+                helpers.excerpt.call(
+                    {
+                        html: html,
+                        custom_excerpt: ''
+                    }
+                )
+            );
+
+        should.exist(rendered);
+        rendered.string.should.equal(expected);
+    });
+
+    it('puts additional space instead of <br> tag', function () {
+        var html = '<p>Testing.<br>Space before this text.<br>And this as well!</p>',
+            expected = 'Testing. Space before this text. And this as well!',
+            rendered = (
+                helpers.excerpt.call(
+                    {
+                        html: html,
+                        custom_excerpt: ''
+                    }
+                )
+            );
+
+        should.exist(rendered);
+        rendered.string.should.equal(expected);
+    });
+
+    it('puts additional space between paragraph in markup generated by Ghost', function () {
+        var html = '<p>put space in excerpt.</p><p></p><p>before this paragraph.</p>' +
+                '<!--kg-card-begin: image--><figure class="kg-card kg-image-card"><img src="/content/images/2019/08/photo.jpg" class="kg-image"></figure><!--kg-card-end: image-->' +
+                '<p>and skip the image.</p><p></p>',
+            expected = 'put space in excerpt.  before this paragraph. and skip the image.',
+            rendered = (
+                helpers.excerpt.call(
+                    {
+                        html: html,
+                        custom_excerpt: ''
+                    }
+                )
+            );
+
+        should.exist(rendered);
+        rendered.string.should.equal(expected);
+    });
 });