Why is the first line stripped for non-HTML notifications

Thanks @eelkeblok for reporting this. I went through the history of the commits around that code block, but I can remember what the purpose has been. I guess we can remove that second part which removes the first line. But we should keen the cleaning up of lines beginning with a space and double new-lines in non-HTML content. Would you agree?

Yes, that was not under dispute, for now (although I wonder if it's a little too aggressive; two newlines are still legitimate, I think (as in, end of a line, and then a blank line). But that may be something for another issue.

Actually, on second thought, double newlines have a use as well, don't they? To create an empty line, for example after a salutation. E.g.

Hi [user:display-name],

[push-object:content]

Best,

The website team

FWIW, I agree with removing the "Strip first line" code unless there a good reason for it that isn't stated here. It's causing me problems with a lot of notifications.

I also agree with @eelkeblok that double line breaks can be a good and necessary thing for readability, so I've made this change:

//          $output = str_replace(["\n ", "\n\n"], "\n", $output, $count);
          $output = str_replace("\n ", "\n", $output, $count);

Meanwhile, I think I found a third issue related to this block of code.

For one content type, I want to send the full body field in the notification. The body field is composed with the Restricted HTML text format, unmodified except that CKEditor5 is the text editor and the "Correct faulty and chopped off HTML" filter is added. The order of the filters is "Limit allowed HTML tags and correct faulty HTML", "Convert URLs into links", "Convert line breaks into HTML (i.e. <br> and <p>)", and "Correct faulty and chopped off HTML."

In the editor and on the page, there's a blank line between each paragraph. But in the notification, it's all one block, with no spaces between the paragraphs. It's hard to read, at least for my aging eyes.

What I think is happening is this:

$output = trim(PlainTextOutput::renderFromHtml($output));

That line is stripping the HTML tags from the content. As a result, what was "</p>\n<p>" becomes just "\n." That means there are no spaces between paragraphs (i.e. it needs to be "\n\n" instead of "\n").

My quick and dirty fix is to add a str_replace just before the "renderFromHtml" line, basically stripping the paragraph tags and replacing them with "\n\n" before the strip_tags is called.

$output = str_replace("</p>\n<p>", "\n\n", $output, $count);
$output = trim(PlainTextOutput::renderFromHtml($output));

I doubt that's the best way to do it, but it's working so I thought I'd add it to the discussion.

Bullets are stripped and not replaced with anything meaningful either, so I added another line to handle that:

        $output = str_replace("</p>\n<p>", "\n\n", $output, $count);
        $output = str_replace("<li>", "  * ", $output, $count);
        $output = trim(PlainTextOutput::renderFromHtml($output));

Maybe to reduce too many consecutive newlines, replace triple newlines with double newlines? Although that is stil quite opinionated and basically means the module doesn't accept anything but a single empty line.

I tried that but got an out-of-memory error. I didn't investigate but I suspect there could be a problem if there are four or more newlines in a row.

jurgenhaas → credited danielspeicher → .

We've discussed this in our team and reviewed all notes and logs for why we've implemented that clean-up in the first place, and we can't find any references anywhere.

So, we've removed that entirely and left the conversion at $output = trim(PlainTextOutput::renderFromHtml($output));. This module is no longer making any assumptions and takes that output as is.

Please have a look if that suits your needs as well.

That looks very sensible. Not sure when I get to work on this again, but I think it will allow me to take HTML mail out of scope of my little project, which is probably a good idea.

Looks like it does the job. Thanks!

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