Skip to content

Add per app notification settings#13598

Open
fredcw wants to merge 1 commit intolinuxmint:masterfrom
fredcw:notifsperapp
Open

Add per app notification settings#13598
fredcw wants to merge 1 commit intolinuxmint:masterfrom
fredcw:notifsperapp

Conversation

@fredcw
Copy link
Contributor

@fredcw fredcw commented Feb 23, 2026

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.

image

fredcw added a commit to fredcw/cinnamon-desktop that referenced this pull request Feb 23, 2026
@mtwebster
Copy link
Member

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
Copy link
Contributor Author

fredcw commented Mar 4, 2026

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

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

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

  1. 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.

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

Good suggestion. Comment added.

  1. Flatpak suffix stripping order — messageTray.js

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

  1. Minor: Trailing whitespace — cs_notifications.py

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants