D10188: Sanitise notification HTML

Fri Feb 2 16:31:58 UTC 2018

davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in notificationsanitizer.cpp:45
> We need a `QXmlStreamEntityResolver` like `KNotification` has otherwise HTML entities like `Ä` (for `Ä`) will error out.

They don't, because that's not what we're parsing.

We will parse &Auml;  which is handled

Sending Ä will get Ä   
Sending Ä will show Ä

Old code is the same.

> broulik wrote in notificationsanitizer.cpp:72
> Don't write `alt` if it doesn't have one?

QmlStreamWriter skips it if the value is empty

> broulik wrote in notificationsengine.cpp:265
> 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.

Oh, I didn't think about that code. You're right.

I'll add a .mid()

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180202/07aa94d6/attachment.html>