Add per app notification settings

[fredcw]

requires: linuxmint/cinnamon-desktop#266

Notes:

1. The new schema and key names were chosen for no other reason than that correspond to the names used by GNOME (org.gnome. ...).

2. When editing the per app notifications in dconf-editor, it says "Keys not defined by a schema". This is because gsettings does not associate relocatable schemas with a path. This is not a problem and I only mention it in case anyone wonders why this isn't the case for GNOME's equivalent relocatable schemas. This is because GNOME's are hard coded into dconf-editor.

[mtwebster]

In the past, I've thought about how an implementation for this might go... this is good for most (well-behaved) apps, but there will be some that don't have a predictable id that can be used... these would be caught in the messageTray - if they're not known, you could add a schema for it and populate it with the defaults. Next time the user went into notification settings it would be there, and further notifications from that source could be turned off. Maybe they'd be better off in their own section also. Something to look at maybe... maybe it's not as much of a concern as I think any more.

I haven't had a chance to try this but I'll attach a Claude code review for now:

 PR #13598 Review: Add per app notification settings
                                                                                                                      
  Files changed: cs_notifications.py (+146), messageTray.js (+34, -21)

  ---
  Critical Issues (1 found)

  1. source.app.id will be undefined — messageTray.js

  In _isAppEnabled (line ~864 of the patched file):

  let appId = source.app.id;

  CinnamonApp does not have an id GObject property — it only has a get_id() method (see src/cinnamon-app.c:117). The
  property is never registered via g_object_class_install_property. Every existing usage in the codebase uses
  app.get_id() (e.g., js/ui/appFavorites.js:31, js/misc/portalHandlers.js:62).

  source.app.id will return undefined, then appId.endsWith(":flatpak") will throw a TypeError, causing the function to
   crash and potentially preventing all notifications from being processed.

  Fix: Change to source.app.get_id().

  ---
  Important Issues (2 found)

  2. New Gio.Settings object created on every notification — messageTray.js

  _isAppEnabled creates a new Gio.Settings object each time a notification arrives:

  const appNotificationSettings = new Gio.Settings({
      schema_id: "org.cinnamon.desktop.notifications.application",
      path: path
  });

  GSettings objects are relatively heavyweight (they involve D-Bus). For a frequent code path like notification
  arrival, this could add unnecessary overhead. Consider caching these settings objects (keyed by settingsId) and
  invalidating on application-children changes.

  3. Disabled notifications are silently destroyed with no trace — messageTray.js

  In the modified _onNotify:

  if (!this._notificationsEnabled || !this._isAppEnabled(source)) {
      notification.destroy(NotificationDestroyedReason.DISMISSED);
      return;
  }

  When per-app notifications are disabled, the notification is destroyed and the user gets zero feedback. Unlike the
  global disable (which is a user's conscious choice in settings), per-app disabling could be confusing if a user
  forgets they disabled an app. This is a design choice rather than a bug, but worth considering whether a log message
   would help debugging.

  ---
  Suggestions (3 found)

  4. Duplicated ID sanitization logic — both files

  The ID sanitization (lowercase → regex replace non-alphanumeric → trim hyphens) is implemented identically in both
  Python and JavaScript:

  Python (cs_notifications.py):
  app_id = app_info.get_id().lower().replace(".desktop", "")
  self.settings_id = re.sub(r'[^a-z0-9]+', '-', app_id).strip('-')

  JavaScript (messageTray.js):
  appId = appId.toLowerCase();
  const settingsId = appId.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '');

  These must stay in sync forever. A comment in each location referencing the other would reduce the risk of them
  diverging. Alternatively, the Python side could document the algorithm once since it's the canonical "writer" side.

  5. Flatpak suffix stripping order — messageTray.js

  if (appId.endsWith(":flatpak")) appId = appId.slice(0, -8);
  if (appId.endsWith(".desktop")) appId = appId.slice(0, -8);

  The Python side only strips .desktop. If a Flatpak app appears in Gio.AppInfo.get_all(), its ID likely won't have
  :flatpak. So the Python sanitization won't strip it, potentially creating a different settings path than the JS side
   for Flatpak apps. Verify that the same Flatpak app produces the same settingsId from both code paths.

  6. Minor: Trailing whitespace — cs_notifications.py

  Line self.set_activatable(True)  has trailing whitespace (visible in the diff). Same with hbox.set_margin_start(8) .

  ---
  _updateState Refactoring Analysis

  The _updateState simplification removes the old logic that drained the notification queue when _notificationsEnabled
   was false. This is correct because that draining is now handled earlier in _onNotify. The simplified version is
  logically equivalent for all remaining cases:

  - HIDDEN + queue non-empty + not busy (or critical): shows notification ✓
  - SHOWN + expired: hides notification ✓
  - The old !_notificationsEnabled drain path is now unreachable since _onNotify blocks disabled notifications from
  entering the queue ✓

  The == to === changes are fine (comparing against constant enum values).

  ---
  Strengths

  - Clean settings UI with search, icons, and toggle switches
  - Smart use of GSettings defaults — only persisting overrides, not all apps
  - Reset All button properly cleans up dconf entries
  - The GNOME-compatible schema naming is a good choice for interoperability
  - The _updateState simplification removes dead code paths correctly

  ---
  Recommended Action

  1. Must fix: source.app.id → source.app.get_id() (Critical — will crash at runtime)
  2. Should fix: Consider caching Gio.Settings objects in _isAppEnabled
  3. Should verify: Flatpak ID normalization produces consistent results across both code paths
  4. Nice to fix: Trailing whitespace in Python file

[fredcw]

if they're not known, you could add a schema for it and populate it with the defaults.

You mean sources not associated with a known app? This would be quite complicated I think. Perhaps we should wait to see how common a problem this is before deciding if it needs implementing?

Thanks for the claude review.

1. source.app.id will be undefined — messageTray.js

Fixed. Turns out cinnamenu was adding .id to the apps so it never failed in testing :p

2. New Gio.Settings object created on every notification — messageTray.js

Fixed. Added a cache for the per app Gio.Settings.

3. Disabled notifications are silently destroyed with no trace — messageTray.js

I don't think this is necessary. Users are more likely to check their per app notification settings than look in .xsession-errors.

4. A comment in each location referencing the other would reduce the risk of them
   diverging

Good suggestion. Comment added.

5. Flatpak suffix stripping order — messageTray.js

Not required. Gio.AppInfo doesn't add ":flatpak" to app_ids.

6. Minor: Trailing whitespace — cs_notifications.py

Fixed.

[fredcw]

@mtwebster

this is good for most (well-behaved) apps, but there will be some that don't have a predictable id that can be used

This would essentially be the same problem as missing notification badges on the GWL icons. Since there haven't been many reports of this being a problem, I don't think it's something we need to be concerned about. If we do find apps for which a source is not identified, I think it's best fixed in notificationDeamon.js so that the notification badges for that app also work.

[mtwebster]

Can you put the 'Enable notifications' switch and the the applications button in their own 'section' with no heading?

Then a new section with "Notification settings" that has the rest of the existing items.

I'm tempted to have the app notifications in their own 'tab' but let's see what feedback we get...

A claude 're-review' (though other than making separate 'sections' in cs_notifications.py I'm ok with everything as is):

 PR #13598 Re-review: Add per app notification settings

  Previous review findings status:
  - Critical #1 (source.app.id → get_id()): Fixed ✓
  - Important #2 (Settings caching): Fixed ✓
  - Important #3 (Silent destroy): Author reasonably declined — acceptable
  - Suggestion #4 (Cross-reference comments): Fixed ✓
  - Suggestion #5 (Flatpak suffix): Author explained — not needed on Python side
  - Suggestion #6 (Trailing whitespace): Fixed ✓

  New/Remaining Issues

  Important Issue (1)

  _updateState no longer checks _notificationsEnabled — queued notifications can leak through

  The old _updateState had:
  let canShowNotification = notificationsPending && this._notificationsEnabled;
  if (this._notificationState == State.HIDDEN) {
      if (canShowNotification) { this._showNotification(); }
      else if (!this._notificationsEnabled) { /* drain queue */ }
  }

  The new version removes both the _notificationsEnabled check and the queue drain:
  if (this._notificationState === State.HIDDEN && this._notificationQueue.length > 0) {
      if (!this._busy || this._notificationQueue[0].urgency === Urgency.CRITICAL) {
          this._showNotification();
      }
  }

  The intent is that _onNotify now blocks disabled notifications from entering the queue. This works for
  new notifications arriving after the setting is changed. But there's a scenario:

  1. Notifications are enabled, a burst arrives, first one is shown (state → SHOWN)
  2. User opens settings, disables notifications globally (or per-app)
  3. First notification hides → _hideNotificationCompleted → _updateState called
  4. Queue is non-empty, _notificationsEnabled is false, but _updateState doesn't check it → shows the
  next queued notification anyway

  The fix is straightforward — add the check back:
  if (this._notificationState === State.HIDDEN && this._notificationQueue.length > 0) {
      if (this._notificationsEnabled && (!this._busy || ...)) {
          this._showNotification();
      }
  }

  The old queue-drain logic (destroying queued notifications when disabled) could also be preserved, but
  is less critical since the queue will naturally empty over time.

  Suggestions (3)

  1. on_reset_all_clicked creates new Settings objects unnecessarily

  The method creates fresh Gio.Settings objects for each overridden app rather than iterating the existing
   AppNotificationRow objects whose switches are already bound. While functionally correct (GSettings
  reflects the same dconf state), iterating the listbox rows would be more direct and would update the UI
  through the existing bindings more cleanly:

  def on_reset_all_clicked(self, button):
      for row in self.listbox.get_children():
          if not row.switch.get_active():
              row.switch.set_active(True)  # triggers update_index → cleans up

  2. The dialog doesn't use run() and has no close button

  AppNotificationsWindow calls self.show_all() in __init__ but never connects to response or delete-event.
   The dialog's title bar close button (from the window manager) will work, but there's no explicit
  "Close" button in the action area. Adding self.add_button(_("Close"), Gtk.ResponseType.CLOSE) and
  connecting self.connect("response", lambda *a: self.destroy()) would be more standard. Currently the
  dialog just stays open with only "Reset All" in the action area.

  3. Minor style: == vs === consistency

  The _updateState refactoring changes == to === for the State comparisons (good), but _onStatusChanged at
   line 884 still uses == for status comparisons. This is just a pre-existing inconsistency, not
  introduced by this PR.

  Strengths

  - Clean, well-structured Settings UI with search filtering
  - Smart use of GSettings defaults — only persisting overrides, keeping dconf clean
  - application-children strv as an index is elegant for the Reset All feature
  - ID sanitization is properly documented with cross-references between JS and Python
  - The _appSettingsCache correctly addresses the previous performance concern
  - Filtering out cinnamon-settings-* from the app list is a nice touch

  Verdict

  Almost ready. The _updateState missing _notificationsEnabled check is the one remaining functional issue
   — queued notifications will display even after the user disables notifications. The dialog UX
  suggestions are minor polish. After fixing the _updateState guard, this is good to merge.

[fredcw]

@mtwebster

Can you put the 'Enable notifications' switch and the the applications button in their own 'section' with no heading?

Done.

Regarding the claude re-review:

_updateState no longer checks _notificationsEnabled — queued notifications can leak through

I think this is rare enough a circumstance and unimportant enough to be not worth adding the extra complication to _updateState()

on_reset_all_clicked creates new Settings objects unnecessarily

The current method has the advantage that it will remove settings for uninstalled apps as well.