Allow custom HTML in HTML Emails

[nickvergessen]

We need to allow custom HTML, so e.g. the activity app can link file names to the file in the web view.
Currently that is not possible because all HTML is escaped.

The question is if we want to do it the simple way like this, or if we want to add a flag/new method that does not automatically-escape the input.

[MorrisJobke]

Then we need to check if all callers properly escape this and document this in the PHPDoc of the method, but I'm fine with that to not clutter the API even more.

[codecov]

Codecov Report

Merging #8026 into master will decrease coverage by <.01%.
The diff coverage is 56.25%.

@@             Coverage Diff              @@
##             master    #8026      +/-   ##
============================================
- Coverage     51.69%   51.69%   -0.01%     
  Complexity    25395    25395              
============================================
  Files          1599     1599              
  Lines         95114    95120       +6     
  Branches       1377     1377              
============================================
+ Hits          49167    49168       +1     
- Misses        45947    45952       +5

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Share20/Manager.php	86.64% <0%> (ø)	236 <0> (ø)	⬇️
core/Controller/LostController.php	80.39% <100%> (ø)	32 <0> (ø)	⬇️
apps/sharebymail/lib/ShareByMailProvider.php	58.8% <50%> (ø)	85 <0> (ø)	⬇️
lib/private/Mail/EMailTemplate.php	60% <54.54%> (-0.45%)	43 <0> (ø)
lib/private/Security/CertificateManager.php	91% <0%> (-3%)	39% <0%> (ø)
lib/private/Server.php	83.28% <0%> (+0.09%)	282% <0%> (ø)	⬇️

[rullzer]

Mmmm yes. lets do it like this for now we can always improve later.

[nickvergessen]

@MorrisJobke can you generate a list for the appstore submissions?

[MorrisJobke]

@MorrisJobke can you generate a list for the appstore submissions?

What do you mean by that?

[nickvergessen]

which apps are using that method and might need adjustment

[MorrisJobke]

addBodyText:

addBodyListItem:

addBodyButton:

[nickvergessen]

Hmm while looking into a patch for the calendar, I realize why a parameter or second method would be better:

// before
$emailTemplate->addBodyText($this->l10n->t('We wanted to inform you that %s has published the calendar »%s«.', [$displayName, $calendarName]));

// after
$emailTemplate->addBodyText(
    htmlspecialchars($this->l10n->t('We wanted to inform you that %s has published the calendar »%s«.', [$displayName, $calendarName])),
    $this->l10n->t('We wanted to inform you that %s has published the calendar »%s«.', [$displayName, $calendarName])
);

basically we need to touch all usages of those methods which use fallback $plain = ''.

Or we automatically encode when plain is the fallback, because that means you want to display the same string and there should not be HTML inside, so we need to special char it.

[MorrisJobke]

Hmm while looking into a patch for the calendar, I realize why a parameter or second method would be better:

I'm fine with that. You need to provide the HTML and the plain text separately. If you want to put HTML in there it should also not present in the plain text (or at least in a different way). So this is okay. Let's go for the "two strings needs to be passed" method I would say.

[juliusknorr]

Or we automatically encode when plain is the fallback, because that means you want to display the same string and there should not be HTML inside, so we need to special char it.

I think a fallback makes sense here - use the text for both and escape the html if only one string is provided.

[nickvergessen]

I think a fallback makes sense here - use the text for both and escape the html if only one string is provided.

That's what we do now, I talked to morris a bit more and explained the use cases.

[MorrisJobke]

Please put this also in the public interface ;)