From 2c8ce1b038dbf3ad59a9ecc64f6fa36a7d7021b2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 24 Mar 2021 17:43:47 +0100 Subject: [PATCH] notifications: Replace existing notifications --- qutebrowser/browser/webengine/notification.py | 29 +++++++++++++++---- tests/end2end/data/prompt/notifications.html | 8 +++++ tests/end2end/features/notifications.feature | 9 ++++++ .../features/test_notifications_bdd.py | 5 ++++ tests/end2end/fixtures/notificationserver.py | 23 ++++++++++----- 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/qutebrowser/browser/webengine/notification.py b/qutebrowser/browser/webengine/notification.py index 208694047..0564cb231 100644 --- a/qutebrowser/browser/webengine/notification.py +++ b/qutebrowser/browser/webengine/notification.py @@ -172,6 +172,19 @@ class DBusNotificationPresenter(QObject): f"Got a message of type {type_str} but expected {expected_type_str}" f"(args: {message.arguments()})") + def _find_matching_id(self, new_notification: "QWebEngineNotification") -> int: + """Find an existing notification to replace. + + If no notification should be replaced or the notification to be replaced was not + found, this returns 0 (as per the notification spec). + """ + if not new_notification.tag(): + return 0 + for notification_id, notification in self._active_notifications.items(): + if notification.matches(new_notification): + return notification_id + return 0 + def _present(self, qt_notification: "QWebEngineNotification") -> None: """Shows a notification over DBus. @@ -184,10 +197,10 @@ class DBusNotificationPresenter(QObject): self._fetch_capabilities() assert self._capabilities is not None - # notification id 0 means 'assign us the ID'. We can't just pass 0 - # because it won't get sent as the right type. - zero = QVariant(0) - zero.convert(QVariant.UInt) + # We can't just pass the int because it won't get sent as the right type. + existing_id = self._find_matching_id(qt_notification) + existing_id_arg = QVariant(existing_id) + existing_id_arg.convert(QVariant.UInt) actions = [] if 'actions' in self._capabilities: @@ -208,7 +221,7 @@ class DBusNotificationPresenter(QObject): QDBus.BlockWithGui, "Notify", "qutebrowser", # application name - zero, # replaces notification id + existing_id_arg, # replaces notification id "qutebrowser", # icon (freedesktop.org icon theme name) # Titles don't support markup, so no need to escape them. qt_notification.title(), @@ -220,6 +233,12 @@ class DBusNotificationPresenter(QObject): self._verify_message(reply, "u", QDBusMessage.ReplyMessage) notification_id = reply.arguments()[0] + if existing_id != 0 and notification_id != existing_id: + raise DBusException(f"Wanted to replace notification {existing_id} but got " + f"new id {notification_id}.") + if notification_id == 0: + raise DBusException("Got invalid notification id 0") + self._active_notifications[notification_id] = qt_notification log.webview.debug(f"Sent out notification {notification_id}") diff --git a/tests/end2end/data/prompt/notifications.html b/tests/end2end/data/prompt/notifications.html index a8867277d..b6d123d12 100644 --- a/tests/end2end/data/prompt/notifications.html +++ b/tests/end2end/data/prompt/notifications.html @@ -46,11 +46,19 @@ let notification = new Notification(str, { body: str }); notification.onshow = function() { console.log("notification shown"); }; } + + function show_multiple_notifications() { + for (let i = 1; i <= 3; i++) { + let notification = new Notification(`i=${i}`, { tag: 'counter' }); + notification.onshow = function() { console.log(`i=${i} notification shown`); }; + } + } + diff --git a/tests/end2end/features/notifications.feature b/tests/end2end/features/notifications.feature index 5f6544005..efa6364a8 100644 --- a/tests/end2end/features/notifications.feature +++ b/tests/end2end/features/notifications.feature @@ -15,6 +15,15 @@ Feature: Notifications Then the javascript message "notification shown" should be logged And a notification with id 1 is presented + @qtwebengine_notifications + Scenario: Replacing existing notifications + When I run :click-element id show-multiple-button + Then the javascript message "i=1 notification shown" should be logged + Then the javascript message "i=2 notification shown" should be logged + Then the javascript message "i=3 notification shown" should be logged + And 1 notification is presented + And notification 1 has title "i=3" + @qtwebengine_notifications Scenario: Notification containing escaped characters Given the notification server supports body markup diff --git a/tests/end2end/features/test_notifications_bdd.py b/tests/end2end/features/test_notifications_bdd.py index 23f7b5e51..dff903c08 100644 --- a/tests/end2end/features/test_notifications_bdd.py +++ b/tests/end2end/features/test_notifications_bdd.py @@ -40,6 +40,11 @@ def notification_presented(notification_server, id_): assert id_ in notification_server.messages +@bdd.then('1 notification is presented') +def notification_presented_single(notification_server): + assert len(notification_server.messages) == 1 + + @bdd.then(bdd.parsers.cfparse('notification {id_:d} has body "{body}"')) def notification_body(notification_server, id_, body): assert notification_server.messages[id_].body == body diff --git a/tests/end2end/fixtures/notificationserver.py b/tests/end2end/fixtures/notificationserver.py index 95e481cd6..a9322b510 100644 --- a/tests/end2end/fixtures/notificationserver.py +++ b/tests/end2end/fixtures/notificationserver.py @@ -35,6 +35,7 @@ class NotificationProperties: title: str body: str + replaces_id: int def _as_uint32(x: int) -> QVariant: @@ -92,7 +93,6 @@ class TestNotificationServer(QObject): values being checked inside test cases. """ assert appname == "qutebrowser" - assert replaces_id == 0 assert icon == "qutebrowser" assert actions == ['default', ''] assert timeout == -1 @@ -101,7 +101,10 @@ class TestNotificationServer(QObject): assert hints['x-qutebrowser-origin'].startswith('http://localhost:') assert hints['desktop-entry'] == 'org.qutebrowser.qutebrowser' - return NotificationProperties(title=title, body=body) + if replaces_id != 0: + assert replaces_id in self.messages + + return NotificationProperties(title=title, body=body, replaces_id=replaces_id) def close(self, notification_id: int) -> None: """Sends a close notification for the given ID.""" @@ -131,12 +134,18 @@ class TestNotificationServer(QObject): # pylint: disable=invalid-name @pyqtSlot(QDBusMessage, result="uint") - def Notify(self, message: QDBusMessage) -> QDBusArgument: - assert message.signature() == 'susssasa{sv}i' - assert message.type() == QDBusMessage.MethodCallMessage + def Notify(self, dbus_message: QDBusMessage) -> QDBusArgument: + assert dbus_message.signature() == 'susssasa{sv}i' + assert dbus_message.type() == QDBusMessage.MethodCallMessage + + message = self._parse_notify_args(*dbus_message.arguments()) + + if message.replaces_id == 0: + message_id = next(self._message_id_gen) + else: + message_id = message.replaces_id + self.messages[message_id] = message - message_id = next(self._message_id_gen) - self.messages[message_id] = self._parse_notify_args(*message.arguments()) return message_id @pyqtSlot(QDBusMessage, result="QStringList")