From 497952611da5afa9b7d79eed0259a46edcf516e1 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Fri, 11 Dec 2020 23:59:38 +0100 Subject: [PATCH] Fix HTML escaping for custom OSD notifications Fixes #618 --- src/context/contextview.cpp | 4 +- src/core/song.cpp | 8 ++ src/core/song.h | 1 + src/core/utilities.cpp | 90 ++++++++------- src/core/utilities.h | 4 +- src/main.cpp | 6 +- src/osd/osdbase.cpp | 122 ++++++++++++++------- src/osd/osdbase.h | 6 +- src/osd/osddbus.cpp | 3 +- src/osd/osdpretty.cpp | 2 +- src/osd/osdpretty.ui | 6 + src/settings/contextsettingspage.cpp | 14 ++- src/settings/contextsettingspage.ui | 24 ++++ src/settings/notificationssettingspage.cpp | 17 +-- src/settings/notificationssettingspage.ui | 24 ++-- 15 files changed, 219 insertions(+), 112 deletions(-) diff --git a/src/context/contextview.cpp b/src/context/contextview.cpp index 9b248b22..e713a9ec 100644 --- a/src/context/contextview.cpp +++ b/src/context/contextview.cpp @@ -486,7 +486,7 @@ void ContextView::UpdateFonts() { void ContextView::SetSong() { label_top_->setStyleSheet(QString("font: %2pt \"%1\"; font-weight: regular;").arg(font_headline_).arg(font_size_headline_)); - label_top_->setText(QString("%1
%2").arg(Utilities::ReplaceMessage(title_fmt_, song_playing_, "
"), Utilities::ReplaceMessage(summary_fmt_, song_playing_, "
"))); + label_top_->setText(QString("%1
%2").arg(Utilities::ReplaceMessage(title_fmt_, song_playing_, "
", true), Utilities::ReplaceMessage(summary_fmt_, song_playing_, "
", true))); label_stop_summary_->clear(); @@ -644,7 +644,7 @@ void ContextView::SetSong() { void ContextView::UpdateSong(const Song &song) { - label_top_->setText(QString("%1
%2").arg(Utilities::ReplaceMessage(title_fmt_, song, "
"), Utilities::ReplaceMessage(summary_fmt_, song, "
"))); + label_top_->setText(QString("%1
%2").arg(Utilities::ReplaceMessage(title_fmt_, song, "
", true), Utilities::ReplaceMessage(summary_fmt_, song, "
", true))); if (action_show_data_->isChecked()) { if (song.filetype() != song_playing_.filetype()) label_filetype_->setText(song.TextForFiletype()); diff --git a/src/core/song.cpp b/src/core/song.cpp index 1670b835..b1ff7de9 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -1427,6 +1427,14 @@ QString Song::PrettyYear() const { } +QString Song::PrettyOriginalYear() const { + + if (effective_originalyear() == -1) return QString(); + + return QString::number(effective_originalyear()); + +} + QString Song::TitleWithCompilationArtist() const { QString title(d->title_); diff --git a/src/core/song.h b/src/core/song.h index 22028300..337c92ea 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -282,6 +282,7 @@ class Song { QString PrettyTitleWithArtist() const; QString PrettyLength() const; QString PrettyYear() const; + QString PrettyOriginalYear() const; QString TitleWithCompilationArtist() const; diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index 523631c8..794178bb 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -924,7 +924,7 @@ QString MacAddress() { } -QString ReplaceMessage(const QString &message, const Song &song, const QString &newline) { +QString ReplaceMessage(const QString &message, const Song &song, const QString &newline, const bool html_escaped) { QRegularExpression variable_replacer("[%][a-z]+[%]"); QString copy(message); @@ -935,7 +935,7 @@ QString ReplaceMessage(const QString &message, const Song &song, const QString & for (match = variable_replacer.match(message, pos) ; match.hasMatch() ; match = variable_replacer.match(message, pos)) { pos = match.capturedStart(); QStringList captured = match.capturedTexts(); - copy.replace(captured[0], ReplaceVariable(captured[0], song, newline)); + copy.replace(captured[0], ReplaceVariable(captured[0], song, newline, html_escaped)); pos += match.capturedLength(); } @@ -943,62 +943,76 @@ QString ReplaceMessage(const QString &message, const Song &song, const QString & if (index_of >= 0) copy = copy.remove(index_of, 3); return copy; + } -QString ReplaceVariable(const QString &variable, const Song &song, const QString &newline) { +QString ReplaceVariable(const QString &variable, const Song &song, const QString &newline, const bool html_escaped) { - QString return_value; - if (variable == "%artist%") { - return song.artist().toHtmlEscaped(); + QString value = variable; + + if (variable == "%title%") { + value = song.PrettyTitle(); } else if (variable == "%album%") { - return song.album().toHtmlEscaped(); + value = song.album(); } - else if (variable == "%title%") { - return song.PrettyTitle().toHtmlEscaped(); + else if (variable == "%artist%") { + value = song.artist(); } else if (variable == "%albumartist%") { - return song.effective_albumartist().toHtmlEscaped(); - } - else if (variable == "%year%") { - return song.PrettyYear().toHtmlEscaped(); - } - else if (variable == "%composer%") { - return song.composer().toHtmlEscaped(); - } - else if (variable == "%performer%") { - return song.performer().toHtmlEscaped(); - } - else if (variable == "%grouping%") { - return song.grouping().toHtmlEscaped(); - } - else if (variable == "%length%") { - return song.PrettyLength().toHtmlEscaped(); - } - else if (variable == "%disc%") { - return return_value.setNum(song.disc()).toHtmlEscaped(); + value = song.effective_albumartist(); } else if (variable == "%track%") { - return return_value.setNum(song.track()).toHtmlEscaped(); + value.setNum(song.track()); + } + else if (variable == "%disc%") { + value.setNum(song.disc()); + } + else if (variable == "%year%") { + value = song.PrettyYear(); + } + else if (variable == "%originalyear%") { + value = song.PrettyOriginalYear(); } else if (variable == "%genre%") { - return song.genre().toHtmlEscaped(); + value = song.genre(); } - else if (variable == "%playcount%") { - return return_value.setNum(song.playcount()).toHtmlEscaped(); + else if (variable == "%composer%") { + value = song.composer(); } - else if (variable == "%skipcount%") { - return return_value.setNum(song.skipcount()).toHtmlEscaped(); + else if (variable == "%performer%") { + value = song.performer(); + } + else if (variable == "%grouping%") { + value = song.grouping(); + } + else if (variable == "%length%") { + value = song.PrettyLength(); } else if (variable == "%filename%") { - return song.basefilename().toHtmlEscaped(); + value = song.basefilename(); + } + else if (variable == "%url%") { + value = song.url().toString(); + } + else if (variable == "%playcount%") { + value.setNum(song.playcount()); + } + else if (variable == "%skipcount%") { + value.setNum(song.skipcount()); + } + else if (variable == "%rating%") { + value = song.PrettyRating(); } else if (variable == "%newline%") { - return QString(newline); + return QString(newline); // No HTML escaping, return immediately. } - //if the variable is not recognized, just return it - return variable; + if (html_escaped) { + value = value.toHtmlEscaped(); + } + return value; + } bool IsColorDark(const QColor &color) { diff --git a/src/core/utilities.h b/src/core/utilities.h index e9cb5f16..a8b7d149 100644 --- a/src/core/utilities.h +++ b/src/core/utilities.h @@ -147,8 +147,8 @@ QString UnicodeToAscii(const QString &unicode); QString MacAddress(); -QString ReplaceMessage(const QString &message, const Song &song, const QString &newline); -QString ReplaceVariable(const QString &variable, const Song &song, const QString &newline); +QString ReplaceMessage(const QString &message, const Song &song, const QString &newline, const bool html_escaped = false); +QString ReplaceVariable(const QString &variable, const Song &song, const QString &newline, const bool html_escaped = false); bool IsColorDark(const QColor &color); diff --git a/src/main.cpp b/src/main.cpp index 67e2a708..a31aeedd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -279,10 +279,10 @@ int main(int argc, char* argv[]) { // Create the tray icon and OSD std::unique_ptr tray_icon(SystemTrayIcon::CreateSystemTrayIcon()); -#ifdef HAVE_DBUS - OSDDBus osd(tray_icon.get(), &app); -#elif defined(Q_OS_MACOS) +#if defined(Q_OS_MACOS) OSDMac osd(tray_icon.get(), &app); +#elif defined(HAVE_DBUS) + OSDDBus osd(tray_icon.get(), &app); #else OSDBase osd(tray_icon.get(), &app); #endif diff --git a/src/osd/osdbase.cpp b/src/osd/osdbase.cpp index 968f1cc5..fb429e2e 100644 --- a/src/osd/osdbase.cpp +++ b/src/osd/osdbase.cpp @@ -130,23 +130,38 @@ void OSDBase::ShowPlaying(const Song &song, const QUrl &cover_url, const QImage QStringList message_parts; QString summary; + bool html_escaped = false; if (use_custom_text_) { - summary = ReplaceMessage(custom_text1_, song); - message_parts << ReplaceMessage(custom_text2_, song); + summary = ReplaceMessage(Type_Summary, custom_text1_, song); + message_parts << ReplaceMessage(Type_Message, custom_text2_, song); } else { summary = song.PrettyTitle(); - if (!song.artist().isEmpty()) + if (!song.artist().isEmpty()) { summary = QString("%1 - %2").arg(song.artist(), summary); - if (!song.album().isEmpty()) + } + if (!song.album().isEmpty()) { message_parts << song.album(); - if (song.disc() > 0) + } + if (song.disc() > 0) { message_parts << tr("disc %1").arg(song.disc()); - if (song.track() > 0) + } + if (song.track() > 0) { message_parts << tr("track %1").arg(song.track()); + } + if (behaviour_ == Pretty) { + summary = summary.toHtmlEscaped(); + html_escaped = true; + } +#if defined(HAVE_DBUS) && !defined(Q_OS_MACOS) + else if (behaviour_ == Native) { + html_escaped = true; + } +#endif } QString message = message_parts.join(", "); + if (html_escaped) message = message.toHtmlEscaped(); if (show_art_) { ShowMessage(summary, message, "notification-audio-play", image); @@ -172,7 +187,7 @@ void OSDBase::Paused() { if (show_on_pause_) { QString summary; if (use_custom_text_) { - summary = ReplaceMessage(custom_text1_, last_song_); + summary = ReplaceMessage(Type_Summary, custom_text1_, last_song_); } else { summary = last_song_.PrettyTitle(); @@ -180,8 +195,10 @@ void OSDBase::Paused() { summary.prepend(" - "); summary.prepend(last_song_.artist()); } + if (behaviour_ == Pretty) { + summary = summary.toHtmlEscaped(); + } } - summary = summary.remove('&').simplified(); if (show_art_) { ShowMessage(summary, tr("Paused"), QString(), last_image_); } @@ -215,7 +232,7 @@ void OSDBase::Stopped() { QString summary; if (use_custom_text_) { - summary = ReplaceMessage(custom_text1_, last_song_); + summary = ReplaceMessage(Type_Summary, custom_text1_, last_song_); } else { summary = last_song_.PrettyTitle(); @@ -223,10 +240,11 @@ void OSDBase::Stopped() { summary.prepend(" - "); summary.prepend(last_song_.artist()); } + if (behaviour_ == Pretty) { + summary = summary.toHtmlEscaped(); + } } - summary = summary.remove('&').simplified(); - if (show_art_) { ShowMessage(summary, tr("Stopped"), QString(), last_image_); } @@ -257,7 +275,17 @@ void OSDBase::VolumeChanged(int value) { if (!show_on_volume_change_) return; - ShowMessage(app_name_, tr("Volume %1%").arg(value)); + QString message = tr("Volume %1%").arg(value); + if (behaviour_ == Pretty) { + message = message.toHtmlEscaped(); + } +#if defined(HAVE_DBUS) && !defined(Q_OS_MACOS) + else if (behaviour_ == Native) { + message = message.toHtmlEscaped(); + } +#endif + + ShowMessage(app_name_, message); } @@ -330,39 +358,51 @@ void OSDBase::RepeatModeChanged(PlaylistSequence::RepeatMode mode) { } -QString OSDBase::ReplaceMessage(const QString &message, const Song &song) { +QString OSDBase::ReplaceMessage(const MessageType type, const QString &message, const Song &song) { - QString newline = "
"; + bool html_escaped = false; + QString newline = ""; - if (message.indexOf("%newline%") != -1) { - // We need different strings depending on notification type - switch (behaviour_) { - case Native: -#ifdef Q_OS_MACOS - newline = "\n"; - break; + // We need different strings depending on notification type + switch (behaviour_) { + case Native: +#if defined(Q_OS_MACOS) + html_escaped = false; + newline = "\n"; + break; +#elif defined(HAVE_DBUS) + switch (type) { + case Type_Summary:{ + html_escaped = false; + newline = ""; + break; + } + case Type_Message: { + html_escaped = true; + newline = "
"; + break; + } + } + break; +#else + // Other OSes doesn't support native notifications. + qLog(Debug) << "Native notifications are not supported on this OS."; + break; #endif -#ifdef Q_OS_LINUX - break; -#endif -#ifdef Q_OS_WIN32 - // Other OS don't support native notifications - qLog(Debug) << "New line not supported by this notification type under Windows"; - newline = ""; - break; -#endif - case TrayPopup: - qLog(Debug) << "New line not supported by this notification type"; - newline = ""; - break; - case Pretty: - default: - // When notifications are disabled, we force the PrettyOSD - break; - } + case TrayPopup: + qLog(Debug) << "New line not supported by this notification type."; + html_escaped = false; + newline = ""; + break; + case Disabled: // When notifications are disabled, we force the PrettyOSD + case Pretty: + html_escaped = true; + newline = "
"; + break; } - return Utilities::ReplaceMessage(message, song, newline); + return Utilities::ReplaceMessage(message, song, newline, html_escaped); + } void OSDBase::ShowPreview(const Behaviour type, const QString &line1, const QString &line2, const Song &song) { @@ -390,5 +430,5 @@ bool OSDBase::SupportsTrayPopups() { } void OSDBase::ShowMessageNative(const QString&, const QString&, const QString&, const QImage&) { - qLog(Warning) << "Not implemented"; + qLog(Warning) << "Native notifications are not supported on this OS."; } diff --git a/src/osd/osdbase.h b/src/osd/osdbase.h index 64422e39..dd62048f 100644 --- a/src/osd/osdbase.h +++ b/src/osd/osdbase.h @@ -83,9 +83,13 @@ class OSDBase : public QObject { void ShowPreview(const Behaviour type, const QString &line1, const QString &line2, const Song &song); private: + enum MessageType { + Type_Summary, + Type_Message, + }; void ShowPlaying(const Song &song, const QUrl &cover_url, const QImage &image, const bool preview = false); void ShowMessage(const QString &summary, const QString &message = QString(), const QString icon = QString("strawberry"), const QImage &image = QImage()); - QString ReplaceMessage(const QString &message, const Song &song); + QString ReplaceMessage(const MessageType type, const QString &message, const Song &song); virtual void ShowMessageNative(const QString &summary, const QString &message, const QString &icon = QString(), const QImage &image = QImage()); private slots: diff --git a/src/osd/osddbus.cpp b/src/osd/osddbus.cpp index b4fab31f..c2dc2caa 100644 --- a/src/osd/osddbus.cpp +++ b/src/osd/osddbus.cpp @@ -144,7 +144,6 @@ void OSDDBus::ShowMessageNative(const QString &summary, const QString &message, QVariantMap hints; QString summary_stripped = summary; summary_stripped = summary_stripped.remove(QRegularExpression("[&\"<>]")).simplified(); - QString message_stripped = message.toHtmlEscaped(); if (!image.isNull()) { if (version_ >= QVersionNumber(1, 2)) { @@ -167,7 +166,7 @@ void OSDDBus::ShowMessageNative(const QString &summary, const QString &message, id = notification_id_; } - QDBusPendingReply reply = interface_->Notify(app_name(), id, icon, summary_stripped, message_stripped, QStringList(), hints, timeout_msec()); + QDBusPendingReply reply = interface_->Notify(app_name(), id, icon, summary_stripped, message, QStringList(), hints, timeout_msec()); QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this); connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher*)), SLOT(CallFinished(QDBusPendingCallWatcher*))); diff --git a/src/osd/osdpretty.cpp b/src/osd/osdpretty.cpp index 24b1088c..d985de81 100644 --- a/src/osd/osdpretty.cpp +++ b/src/osd/osdpretty.cpp @@ -343,7 +343,7 @@ void OSDPretty::paintEvent(QPaintEvent *) { } -void OSDPretty::SetMessage(const QString& summary, const QString& message, const QImage &image) { +void OSDPretty::SetMessage(const QString &summary, const QString& message, const QImage &image) { if (!image.isNull()) { QImage scaled_image = image.scaled(kMaxIconSize, kMaxIconSize, Qt::KeepAspectRatio, Qt::SmoothTransformation); diff --git a/src/osd/osdpretty.ui b/src/osd/osdpretty.ui index afccbcee..0931659d 100644 --- a/src/osd/osdpretty.ui +++ b/src/osd/osdpretty.ui @@ -53,6 +53,9 @@ 16777215 + + Qt::RichText + true @@ -72,6 +75,9 @@ 16777215 + + Qt::RichText + true diff --git a/src/settings/contextsettingspage.cpp b/src/settings/contextsettingspage.cpp index 40f976b0..04f11ea9 100644 --- a/src/settings/contextsettingspage.cpp +++ b/src/settings/contextsettingspage.cpp @@ -75,21 +75,23 @@ ContextSettingsPage::ContextSettingsPage(SettingsDialog* dialog) : SettingsPage( // Create and populate the helper menus QMenu *menu = new QMenu(this); - menu->addAction(ui_->action_albumartist); - menu->addAction(ui_->action_artist); - menu->addAction(ui_->action_album); menu->addAction(ui_->action_title); + menu->addAction(ui_->action_album); + menu->addAction(ui_->action_artist); + menu->addAction(ui_->action_albumartist); + menu->addAction(ui_->action_track); + menu->addAction(ui_->action_disc); menu->addAction(ui_->action_year); + menu->addAction(ui_->action_originalyear); menu->addAction(ui_->action_composer); menu->addAction(ui_->action_performer); menu->addAction(ui_->action_grouping); + menu->addAction(ui_->action_filename); + menu->addAction(ui_->action_url); menu->addAction(ui_->action_length); - menu->addAction(ui_->action_disc); - menu->addAction(ui_->action_track); menu->addAction(ui_->action_genre); menu->addAction(ui_->action_playcount); menu->addAction(ui_->action_skipcount); - menu->addAction(ui_->action_filename); menu->addSeparator(); menu->addAction(ui_->action_newline); ui_->context_exp_chooser1->setMenu(menu); diff --git a/src/settings/contextsettingspage.ui b/src/settings/contextsettingspage.ui index 895772fb..4a8e593a 100644 --- a/src/settings/contextsettingspage.ui +++ b/src/settings/contextsettingspage.ui @@ -473,6 +473,30 @@ Add song filename + + + %url% + + + Add song URL + + + + + %rating% + + + Add song rating + + + + + %originalyear% + + + Add song original year tag + + diff --git a/src/settings/notificationssettingspage.cpp b/src/settings/notificationssettingspage.cpp index e55d4e08..ec8c734b 100644 --- a/src/settings/notificationssettingspage.cpp +++ b/src/settings/notificationssettingspage.cpp @@ -56,7 +56,7 @@ class QHideEvent; class QShowEvent; -NotificationsSettingsPage::NotificationsSettingsPage(SettingsDialog* dialog) +NotificationsSettingsPage::NotificationsSettingsPage(SettingsDialog *dialog) : SettingsPage(dialog), ui_(new Ui_NotificationsSettingsPage), pretty_popup_(new OSDPretty(OSDPretty::Mode_Draggable)) { ui_->setupUi(this); @@ -69,23 +69,24 @@ NotificationsSettingsPage::NotificationsSettingsPage(SettingsDialog* dialog) // Create and populate the helper menus QMenu *menu = new QMenu(this); - menu->addAction(ui_->action_artist); - menu->addAction(ui_->action_album); menu->addAction(ui_->action_title); + menu->addAction(ui_->action_album); + menu->addAction(ui_->action_artist); menu->addAction(ui_->action_albumartist); + menu->addAction(ui_->action_disc); + menu->addAction(ui_->action_track); menu->addAction(ui_->action_year); + menu->addAction(ui_->action_originalyear); + menu->addAction(ui_->action_genre); menu->addAction(ui_->action_composer); menu->addAction(ui_->action_performer); menu->addAction(ui_->action_grouping); menu->addAction(ui_->action_length); - menu->addAction(ui_->action_disc); - menu->addAction(ui_->action_track); - menu->addAction(ui_->action_genre); + menu->addAction(ui_->action_filename); + menu->addAction(ui_->action_url); menu->addAction(ui_->action_playcount); menu->addAction(ui_->action_skipcount); - menu->addAction(ui_->action_filename); menu->addAction(ui_->action_rating); - menu->addAction(ui_->action_score); menu->addSeparator(); menu->addAction(ui_->action_newline); ui_->notifications_exp_chooser1->setMenu(menu); diff --git a/src/settings/notificationssettingspage.ui b/src/settings/notificationssettingspage.ui index d244a19c..bd4b2479 100644 --- a/src/settings/notificationssettingspage.ui +++ b/src/settings/notificationssettingspage.ui @@ -482,14 +482,6 @@ Add song rating - - - %score% - - - Add song auto score - - %newline% @@ -506,6 +498,22 @@ Add song filename + + + %url% + + + Add song URL + + + + + %originalyear% + + + Add song original year tag + + notifications_none