Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message preview in message list sometimes gets misrepresented #8222

Open
jancborchardt opened this issue Mar 14, 2023 · 5 comments
Open

Message preview in message list sometimes gets misrepresented #8222

jancborchardt opened this issue Mar 14, 2023 · 5 comments
Assignees
Labels
1. to develop bug design feature: HTML mails skill:frontend Issues and PRs that require JavaScript/Vue/styling development skills

Comments

@jancborchardt
Copy link
Member

Steps to reproduce

  1. View the message list in any mailbox
  2. Sometimes it looks off (listed below)

Expected behavior

The preview text should look like the text when you open the mail.

Actual behavior

Sometimes the text includes:

  • ALL-CAPS of text which is normally written in the original mail (like for our own notifications, see screenshot below)
  • Alt-text of images in [Brackets], unnecessary inclusion (see below)
  • Links rendered in [Markdown style](https://example.com), also unnecessary to include the link part, only the text is enough.

image

Mail app version

Current on cloud.nc.com

Mailserver or service

No response

Operating system

No response

PHP engine version

None

Web server

None

Database

None

Additional info

No response

@jancborchardt jancborchardt added bug 1. to develop feature: HTML mails skill:frontend Issues and PRs that require JavaScript/Vue/styling development skills labels Mar 14, 2023
@GretaD
Copy link
Contributor

GretaD commented Mar 15, 2023

Can you reproduce locally with the latest main?

Yes, it is there, but i am having a look if the text insde [] is added by a sysadmin setup like [external] or mail app adds it

@hamza221 hamza221 self-assigned this Mar 16, 2023
@hamza221
Copy link
Contributor

hamza221 commented Mar 16, 2023

After a bit of investigation, the issue comes from the html2text package. The link problem is easily fixable since html2text provides an option to hide markdown-style links. when it comes to image alt text, If mtibben/html2text#116 gets merged the problem will be fixed, if not a workaround would also be possible. I'm not sure I understood

  • ALL-CAPS of text which is normally written in the original mail (like for our own notifications, see screenshot below)

correctly, So we should avoid having all-caps text even if it's in the original mail?

@hamza221
Copy link
Contributor

Also, In the case of a list the preview looks like this
image
Should we keep it the way it is, or remove the asterisks

@ChristophWurst
Copy link
Member

In the specific case of a suspicious_login email (and likely other nextcloud emails) there are two alternative parts in the message. A text one and a HTML one. I probably makes most sense to take the text part for the preview, not the HTML.

Bildschirmfoto vom 2023-03-16 14-15-03

@ChristophWurst
Copy link
Member

I probably makes most sense to take the text part for the preview, not the HTML.

By flipping the order of the two ifs at

$htmlBody = ($htmlBodyId !== null) ? $part->getBodyPart($htmlBodyId) : null;
if (!empty($htmlBody)) {
$mimeHeaders = $part->getMimeHeader($htmlBodyId, Horde_Imap_Client_Data_Fetch::HEADER_PARSE);
if ($enc = $mimeHeaders->getValue('content-transfer-encoding')) {
$structure->setTransferEncoding($enc);
$structure->setContents($htmlBody);
$htmlBody = $structure->getContents();
}
$html = new Html2Text($htmlBody);
return new MessageStructureData(
$hasAttachments,
trim($html->getText()),
$isImipMessage,
$isEncrypted,
);
}
$textBody = $part->getBodyPart($textBodyId);
if (!empty($textBody)) {
$mimeHeaders = $part->getMimeHeader($textBodyId, Horde_Imap_Client_Data_Fetch::HEADER_PARSE);
if ($enc = $mimeHeaders->getValue('content-transfer-encoding')) {
$structure->setTransferEncoding($enc);
$structure->setContents($textBody);
$textBody = $structure->getContents();
}
return new MessageStructureData(
$hasAttachments,
$textBody,
$isImipMessage,
$isEncrypted,
);
}
text parts would that precedence over HTML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop bug design feature: HTML mails skill:frontend Issues and PRs that require JavaScript/Vue/styling development skills
Projects
Status: 🏗️ In progress
Status: 🧭 Planning evaluation / ideas
Development

No branches or pull requests

4 participants