Ghost: No space between paragraphs in excerpt

[naz]

Issue Summary

It occurs in Ghost 2.15.0 with default Casper theme.

To Reproduce

1. Create a post with two paragraphs that would produce html similar to:

<p>Testing.</p><p>Space before this text.</p>

1. Publish the post.
2. Go to blog's main page and check the excerpt of the newly published post.

The text of the excerpt reads: Testing.Space before this text. but should be Testing. Space before this text.

Technical details:

• Ghost Version: 2.15.0
• Node Version: v10.15.1
• Browser/OS: Ubuntu 18.04
• Database: MySQL

[kirrg001]

Is that a critical bug?

[naz]

Not critical, just something @JohnONolan asked about today.

[kirrg001]

Okay if not critical, it has no priority for today 👍

[naz]

If somebody picks this up, here is a PR with a test case that describes the issue -https://github.com/TryGhost/Ghost/pull/10532 :wink:

[rmhassan]

I want to do this but can anyone guide me where I can solve this issue?

[naz]

Hey @Hassanraja447 :wave: Would start with looking at the test case in the PR I mentioned above and work your way into the place where it's most suitable to do the change. From a very quick glance seems like https://github.com/TryGhost/Ghost/blob/master/core/server/data/meta/excerpt.js is a good candidate :wink:

[rmhassan]

Hey @Hassanraja447 👋 Would start with looking at the test case in the PR I mentioned above and work your way into the place where it's most suitable to do the change. From a very quick glance seems like https://github.com/TryGhost/Ghost/blob/master/core/server/data/meta/excerpt.js is a good candidate 😉

Thank you I will give it a shot

[NCHydr4]

Hello @gargol you are right the issue was on the https://github.com/TryGhost/Ghost/blob/master/core/server/data/meta/excerpt.js file line 9 - 10

excerpt = excerpt.replace(/<\/?[^>]+>/gi, ' ');    <--- I added a space.
excerpt = excerpt.replace(/(\r\n|\n|\r)+/gm, ' ');

Now after the restart there is a space betweed
Testing. Space before this text.

[naz]

@NCHydr4 this regexp catches all of html tags, and would produce extra spaces in places we don't really want, e.g. <a href...>link</a> would give you extra space. We probably want to be very specific about the type of tags that are transformed into additional space, <p></p> would be a good start :+1: Alternatively, could substitute all tags with space, and dedup double spaces :thinking: Think this case needs some playing around to find the best approach :wink:

[NCHydr4]

@gargol Thx for your reply, I guess adding something likeexcerpt = excerpt.replace(/<\/p>/g," "); before the two excerpt could be a good start (I've tried it and it works).
I dont know if it is neccesary to add a replace rule for the

.

    excerpt = excerpt.replace(/<\/p>/g,' ');
    excerpt = excerpt.replace(/<\/?[^>]+>/gi, '');
    excerpt = excerpt.replace(/(\r\n|\n|\r)+/gm, ' ');

I dont know if that could be a real solution, I keep looking for something more suitable.

[kscott5]

@nchydr4. defensive code is a good thing with a mature codebase. A unit test helps here if possible.

I know javascript unit test frameworks exists.

I often use speech or voice text. It's a nice change today.

@gargol

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[JohnONolan]

Not stale.

@gargol would you be able to choose an approach and get this closed?

[lunaticmonk]

@gargol @JohnONolan How about using an array of the regex for which we want to introduce a whitespace?
Somewhat like this:

const whiteSpaceExcerptRegex = new Array(/<\/p>/g);
for (const regex of whiteSpaceExcerptRegex) {
    excerpt = excerpt.replace(regex, ' ');
}

I tried this approach. Seems to work fine. Let me know your thoughts on this.

[naz]

@lunaticmonk in #11053, took the most simplistic approach to the issue and added a single substitution rule for closing </p> tag and line break <br> - that's the case that covers 99% of HTML that comes out of internal mobiledoc format. In case there are more cases surfacing in the future more cases can be added to the grouping in the RegExp: /(<\/p>|<br>|other_cases...)/