Decouple notification closed reporting from closing of notifications

docs/desktop-notifications.rst

@@ -54,9 +54,12 @@ limit to avoid Denial-of-Service attacks). The size of the payload must be no
 longer than ``2048`` bytes, *before being encoded*.
 
 Both the ``title`` and ``body`` payloads must be either UTF-8 encoded plain
-text with no embedded escape codes, or UTF-8 text that is Base64 encoded, in
+text with no embedded escape codes, or UTF-8 text that is :rfc:`base64 <4648>`
-which case there must be an ``e=1`` key in the metadata to indicate the payload
+encoded, in which case there must be an ``e=1`` key in the metadata to indicate
-is Base64 encoded.
+the payload is :rfc:`base64 <4648>` encoded.
+
+Being informed when user activates the notification
+-------------------------------------------------------
 
 When the user clicks the notification, a couple of things can happen, the
 terminal emulator can focus the window from which the notification came, and/or
@@ -93,6 +96,24 @@ to display it based on what it does understand.
    Similarly, features such as scheduled notifications could be added in future
    revisions.
 
+Being informed when a notification is closed
+------------------------------------------------
+
+If you wish to be informed when a notification is closed, you can specify
+``c=1`` when sending the notification. For example::
+
+    <OSC> 99 ; i=mynotification : c=1 ; hello world <terminator>
+
+Then, the terminal will send the following
+escape code to inform when the notification is closed::
+
+    <OSC> 99 ; i=mynotification : p=close ; <terminator>
+
+If no notification id was specified ``i=0`` will be used.
+If ``a=report`` is specified and the notification is activated/clicked on
+then both the activation report and close notification are sent.
+
+
 Closing an existing notification
 ----------------------------------
 
@@ -103,24 +124,9 @@ To close a previous notification, send::
 
     <OSC> i=<notification id> : p=close ; <terminator>
 
-This will close a previous notification with the specified id. If you want a
+This will close a previous notification with the specified id. If no such
-notification when closing succeeds, send the following instead::
+notification exists (perhaps because it was already closed or it was activated)
-
+then the request is ignored.
-    <OSC> i=<notification id> : p=close ; notify <terminator>
-
-Then, the terminal will respond with::
-
-    <OSC> i=<notification id> : p=close ; <terminator>  # notification was closed
-    or
-    <OSC> i=<notification id> : p=close ; ENOENT ; <terminator>  # notification did not exist
-
-This escape code is sent by the terminal if the notification is closed or a
-notification with the specified identifier does not exist. If the notification
-is activated or closed before the close request is received, then the notification does
-not exist as far as the terminal is concerned, and an ``ENOENT`` response is
-sent. To detect activation, before close, enable reporting with ``a=report``
-when creating the notification.
-
 
 Querying for support
 -------------------------
@@ -187,7 +193,7 @@ Key      Value                 Default    Description
 ``d``    ``0`` or ``1``        ``1``      Indicates if the notification is
                                           complete or not.
 
-``e``    ``0`` or ``1``        ``0``      If set to ``1`` means the payload is Base64 encoded UTF-8,
+``e``    ``0`` or ``1``        ``0``      If set to ``1`` means the payload is :rfc:`base64 <4648>` encoded UTF-8,
                                           otherwise it is plain UTF-8 text with no C0 control codes in it
 
 ``i``    ``[a-zA-Z0-9-_+.]``   ``0``      Identifier for the notification. Make these globally unqiue,
@@ -206,8 +212,11 @@ Key      Value                 Default    Description
                                           and not visible to the user, for example, because it is in an inactive tab or
                                           its OS window is not currently active.
                                           ``always`` is the default and always honors the request.
+
 ``u``    ``0, 1 or 2``         ``unset``  The *urgency* of the notification. ``0`` is low, ``1`` is normal and ``2`` is critical.
                                           If not specified normal is used.
+
+``c``    ``0`` or ``1``        ``0``      When non-zero an escape code is sent to the application when the notification is closed.
 =======  ====================  ========== =================
 
 

docs/graphics-protocol.rst

@@ -233,7 +233,7 @@ This is a so-called *Application Programming Command (APC)*. Most terminal
 emulators ignore APC codes, making it safe to use.
 
 The control data is a comma-separated list of ``key=value`` pairs.  The payload
-is arbitrary binary data, base64-encoded to prevent interoperation problems
+is arbitrary binary data, :rfc:`base64 <4648>` encoded to prevent interoperation problems
 with legacy terminals that get confused by control codes within an APC code.
 The meaning of the payload is interpreted based on the control data.
 
@@ -294,7 +294,8 @@ compression is supported, which is specified using ``o=z``. For example::
     <ESC>_Gf=24,s=10,v=20,o=z;<payload><ESC>\
 
 This is the same as the example from the RGB data section, except that the
-payload is now compressed using deflate (this occurs prior to base64-encoding).
+payload is now compressed using deflate (this occurs prior to
+:rfc:`base64 <4648>` encoding).
 The terminal emulator will decompress it before rendering. You can specify
 compression for any format. The terminal emulator will decompress before
 interpreting the pixel data.
@@ -366,7 +367,7 @@ Remote clients, those that are unable to use the filesystem/shared memory to
 transmit data, must send the pixel data directly using escape codes. Since
 escape codes are of limited maximum length, the data will need to be chunked up
 for transfer. This is done using the ``m`` key. The pixel data must first be
-base64 encoded then chunked up into chunks no larger than ``4096`` bytes. All
+:rfc:`base64 <4648>` encoded then chunked up into chunks no larger than ``4096`` bytes. All
 chunks, except the last, must have a size that is a multiple of 4. The client
 then sends the graphics escape code as usual, with the addition of an ``m`` key
 that must have the value ``1`` for all but the last chunk, where it must be

kitty/notifications.py

@@ -14,6 +14,8 @@ from .types import run_once
 from .typing import WindowType
 from .utils import get_custom_window_icon, log_error, sanitize_control_codes
 
+debug_desktop_integration = False
+
 
 class Urgency(Enum):
     Low: int = 0
@@ -30,7 +32,7 @@ class PayloadType(Enum):
 
     @property
     def is_text(self) -> bool:
-        return self in (PayloadType.title, PayloadType.body, PayloadType.close, PayloadType.query)
+        return self in (PayloadType.title, PayloadType.body)
 
 
 class OnlyWhen(Enum):
@@ -132,7 +134,7 @@ class NotificationCommand:
     actions: FrozenSet[Action] = frozenset((Action.focus,))
     only_when: OnlyWhen = OnlyWhen.unset
     urgency: Optional[Urgency] = None
-    close_response_requested: bool = False
+    close_response_requested: Optional[bool] = None
 
     # payload handling
     current_payload_type: PayloadType = PayloadType.title
@@ -194,6 +196,8 @@ class NotificationCommand:
                 elif k == 'u':
                     with suppress(Exception):
                         self.urgency = Urgency(int(v))
+                elif k == 'c':
+                    self.close_response_requested = v != '0'
         if not prev.done and prev.identifier == self.identifier:
             self.actions = prev.actions.union(self.actions)
             self.title = prev.title
@@ -202,6 +206,8 @@ class NotificationCommand:
                 self.only_when = prev.only_when
             if self.urgency is None:
                 self.urgency = prev.urgency
+            if self.close_response_requested is None:
+                self.close_response_requested = prev.close_response_requested
 
         return payload_type, payload_is_encoded
 
@@ -306,11 +312,16 @@ class FreeDesktopIntegration(DesktopIntegration):
 
     def close_notification(self, desktop_notification_id: int) -> bool:
         from .fast_data_types import dbus_close_notification
-        return dbus_close_notification(desktop_notification_id)
+        close_succeeded = dbus_close_notification(desktop_notification_id)
+        if debug_desktop_integration:
+            log_error(f'Close request for {desktop_notification_id=} {"succeeded" if close_succeeded else "failed"}')
+        return close_succeeded
 
     def dispatch_event_from_desktop(self, *args: Any) -> None:
         event_type: str = args[0]
         dbus_notification_id: int = args[1]
+        if debug_desktop_integration:
+            log_error(f'Got notification event from desktop: {args=}')
         if event_type == 'created':
             self.notification_manager.notification_created(dbus_notification_id)
         elif event_type == 'activation_token':
@@ -334,7 +345,10 @@ class FreeDesktopIntegration(DesktopIntegration):
         if icon is True:
             icf = get_custom_window_icon()[1] or logo_png_file
         from .fast_data_types import dbus_send_notification
-        return dbus_send_notification(application, icf, title, body, 'Click to see changes', timeout, urgency.value)
+        desktop_notification_id = dbus_send_notification(application, icf, title, body, 'Click to see changes', timeout, urgency.value)
+        if debug_desktop_integration:
+            log_error(f'Created notification with {desktop_notification_id=}')
+        return desktop_notification_id
 
 
 class UIState(NamedTuple):
@@ -422,12 +436,13 @@ class NotificationManager:
 
     def notification_activated(self, desktop_notification_id: int) -> None:
         if n := self.in_progress_notification_commands.get(desktop_notification_id):
-            self.purge_notification(n)
+            if not n.close_response_requested:
+                self.purge_notification(n)
             if n.focus_requested:
                 self.channel.focus(n.channel_id, n.activation_token)
             if n.report_requested:
-                if n.identifier:
+                ident = n.identifier or '0'
-                    self.channel.send(n.channel_id, f'99;i={n.identifier};')
+                self.channel.send(n.channel_id, f'99;i={ident};')
             if n.on_activation:
                 try:
                     n.on_activation(n)
@@ -505,23 +520,13 @@ class NotificationManager:
             self.channel.send(channel_id, f'99;{i}p=?;a={actions}:o={when}:u={urgency}:p={p}')
             return None
         if payload_type is PayloadType.close:
-            if payload_is_encoded:
-                from base64 import standard_b64decode
-                try:
-                    payload = standard_b64decode(payload).decode('utf-8')
-                except Exception:
-                    self.log('Malformed OSC 99 close command: payload is not base64 encoded UTF-8 text')
             if cmd.identifier:
                 to_close = self.in_progress_notification_commands_by_client_id.get(cmd.identifier)
                 if to_close:
-                    if payload == 'notify':
-                        to_close.close_response_requested = True
                     if not self.desktop_integration.close_notification(to_close.desktop_notification_id):
                         if to_close.close_response_requested:
                             self.send_closed_response(to_close.channel_id, to_close.identifier)
                         self.purge_notification(to_close)
-                else:
-                    self.send_closed_response(channel_id, cmd.identifier, not_found=True)
             return None
 
         if payload_type is PayloadType.unknown:
@@ -531,9 +536,8 @@ class NotificationManager:
         cmd.set_payload(payload_type, payload_is_encoded, payload, prev_cmd)
         return cmd
 
-    def send_closed_response(self, channel_id: int, client_id: str, not_found: bool = False) -> None:
+    def send_closed_response(self, channel_id: int, client_id: str) -> None:
-        trailer = 'ENOENT;' if not_found else ''
+        self.channel.send(channel_id, f'99;i={client_id}:p=close;')
-        self.channel.send(channel_id, f'99;i={client_id}:p=close;{trailer}')
 
     def purge_notification(self, cmd: NotificationCommand) -> None:
         self.in_progress_notification_commands_by_client_id.pop(cmd.identifier, None)

kitty_tests/notifications.py

@@ -93,7 +93,7 @@ def do_test(self: 'TestNotifications') -> None:
         n = di.notifications[which]
         di.close_notification(n['id'])
 
-    def assert_events(focus=True, close=0, report='', close_response='', enoent=False):
+    def assert_events(focus=True, close=0, report='', close_response=''):
         self.ae(ch.focus_events, [''] if focus else [])
         if report:
             self.assertIn(f'99;i={report};', ch.responses)
@@ -102,8 +102,7 @@ def do_test(self: 'TestNotifications') -> None:
                 m = re.match(r'99;i=[a-z0-9]+;', r)
                 self.assertIsNone(m, f'Unexpectedly found report response: {r}')
         if close_response:
-            t = 'ENOENT;' if enoent else ''
+            self.assertIn(f'99;i={close_response}:p=close;', ch.responses)
-            self.assertIn(f'99;i={close_response}:p=close;{t}', ch.responses)
         else:
             for r in ch.responses:
                 m = re.match(r'99;i=[a-z0-9]+:p=close;', r)
@@ -154,6 +153,12 @@ def do_test(self: 'TestNotifications') -> None:
     assert_events(report='x')
     reset()
 
+    h('a=report;title')
+    self.ae(di.notifications, [n()])
+    activate()
+    assert_events(report='0')
+    reset()
+
     h('d=0:i=y;title')
     h('d=1:i=y:p=xxx;title')
     self.ae(di.notifications, [n()])
@@ -165,28 +170,29 @@ def do_test(self: 'TestNotifications') -> None:
     close()
     assert_events(focus=False, close=True)
     reset()
-    h('i=c;title')
+    h('i=c:c=1;title')
     self.ae(di.notifications, [n()])
     h('i=c:p=close')
     self.ae(di.notifications, [n()])
-    assert_events(focus=False, close=True)
+    assert_events(focus=False, close=True, close_response='c')
     reset()
-    h('i=c;title')
+    h('i=c:c=1;title')
-    h('i=c:p=close;notify')
+    h('i=c:p=close')
     self.ae(di.notifications, [n()])
     assert_events(focus=False, close=True, close_response='c')
     reset()
     h('i=c;title')
     activate()
-    h('i=c:p=close;notify')
+    close()
+    h('i=c:p=close')
     self.ae(di.notifications, [n()])
-    assert_events(focus=True, close_response='c', enoent=True)
+    assert_events(focus=True, close=True)
     reset()
-    h('i=c:a=report;title')
+    h('i=c:a=report:c=1;title')
     activate()
-    h('i=c:p=close;notify')
+    h('i=c:p=close')
     self.ae(di.notifications, [n()])
-    assert_events(focus=True, report='c', close_response='c', enoent=True)
+    assert_events(focus=True, report='c', close=True, close_response='c')
     reset()
 
     h(';title')