broulik added a comment.
Thanks for taking care of this. INLINE COMMENTS > notificationsanitizer.cpp:45 > + > + QXmlStreamReader r(QStringLiteral("<html>") + t + > QStringLiteral("</html>")); > + QString result; We need a `QXmlStreamEntityResolver` like `KNotification` has otherwise HTML entities like `Ä` (for `Ä`) will error out. > notificationsanitizer.cpp:72 > + > + out.writeAttribute(QStringLiteral("alt"), alt); > + } Don't write `alt` if it doesn't have one? > notificationsanitizer.h:2 > +/* > + * Copyright (C) 2017 David Edmundson <davidedmund...@kde.org> > + * 2018 > notificationsengine.cpp:265 > QString bodyFinal = (partOf == 0 ? body : _body); > - // First trim whitespace from beginning and end > - bodyFinal = bodyFinal.trimmed(); > - // Now replace all \ns with <br/> > - bodyFinal = bodyFinal.replace(QLatin1String("\n"), > QLatin1String("<br/>")); > - // Now remove all inner whitespace (\ns are already <br/>s > - bodyFinal = bodyFinal.simplified(); > - // Finally, check if we don't have multiple <br/>s following, > - // can happen for example when "\n \n" is sent, this replaces > - // all <br/>s in succsession with just one > - > bodyFinal.replace(QRegularExpression(QStringLiteral("<br/>\\s*<br/>(\\s|<br/>)*")), > QLatin1String("<br/>")); > - // This fancy RegExp escapes every occurence of & since QtQuick Text > will blatantly cut off > - // text where it finds a stray ampersand. > - // Only &{apos, quot, gt, lt, amp}; as well as { character > references will be allowed > - > bodyFinal.replace(QRegularExpression(QStringLiteral("&(?!(?:apos|quot|[gl]t|amp);|#)")), > QLatin1String("&")); > - // The Text.StyledText format handles only html3.2 stuff and ' is > html4 stuff > - // so we need to replace it here otherwise it will not render at all. > - bodyFinal.replace(QLatin1String("'"), QChar('\'')); > + bodyFinal = NotificationSanitizer::parse(bodyFinal); > Won't you end up with piles of `<html>` tags since `_body` is the body text of the notification it would group to. <html> <html> old notification </html> new notification </html> Not that it really matters, though. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10188 To: davidedmundson, #plasma, fvogt Cc: broulik, aacid, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart