Skip to content

Fix several bugs: regex flags, implicit globals, crashes, ngettext, undefined icon#13643

Open
KAMI911 wants to merge 8 commits intolinuxmint:masterfrom
KAMI911:bugfixes/code-review-2024
Open

Fix several bugs: regex flags, implicit globals, crashes, ngettext, undefined icon#13643
KAMI911 wants to merge 8 commits intolinuxmint:masterfrom
KAMI911:bugfixes/code-review-2024

Conversation

@KAMI911
Copy link

@KAMI911 KAMI911 commented Mar 14, 2026

Summary

This PR contains 8 small, independent bug fixes found during a code review. Each fix is a separate commit.


1. util: fixupPCIDescription: fix missing global flag in regex replacements

Two regex replacements in fixupPCIDescription() were missing the /g flag, causing only the first occurrence to be replaced:

  • /[_,]//[_,]/g — replace all underscores/commas with spaces
  • /\([\s\S][^\(\)]{2,}\)/ → same with /g — strip all parenthesized segments, not just the first

2. keyboardManager: fix implicit global variable declarations

Five constants at the top of the file (DESKTOP_INPUT_SOURCES_SCHEMA, KEY_INPUT_SOURCES, KEY_KEYBOARD_OPTIONS, KEY_PER_WINDOW, KEY_SOURCE_LAYOUTS) were assigned without var, making them implicit globals that pollute the global scope. Added var to match the style used elsewhere in the file.


3. notificationDaemon: add default case for unknown urgency in icon switch

The switch (hints.urgency) in _iconForNotificationData() had no default branch, leaving stockIcon as undefined for any urgency value outside LOW/NORMAL/CRITICAL. This silently created a broken St.Icon with icon_name: undefined. Added a default case falling back to the information icon.


4. notificationDaemon: fix crash in _expireNotification when notification is null

NotifyAsync() adds ndata to _expireNotifications and starts the expire timer before the async GetConnectionUnixProcessID D-Bus call completes. ndata.notification is only assigned later inside _notifyForSource(). If the timer fires during that window, calling ndata.notification.destroy() on undefined throws a TypeError. Added a null guard; if the notification object doesn't exist yet, the orphaned entry is removed and the timer is restarted for the next item.


5. windowManager: fix ngettext missing count argument in display-change dialog

ngettext(singular, plural) requires a third argument — the count used to select the correct plural form. Without it the function always returns the singular string (e.g. "Reverting in 5 second." instead of "Reverting in 5 seconds."). Fixed to match the pattern used in endSessionDialog.js.


6. cinnamonDBus: fix crash in activateCallback when xlet is not found

_getXletObject() returns null when no applet/desklet/extension matches the given uuid/instance_id. The next line called null[callback].bind(null), throwing a TypeError. This happens when cinnamon-settings calls a callback for an xlet that was removed without a shell restart. Added a null guard with a warning log.


7. cinnamonDBus: fix crash in updateSetting when instance_id is not found

updateSetting() already guarded against an unknown uuid, but not against an unknown instance_id within a valid uuid. Accessing uuids[uuid][instance_id].remoteUpdate() when instance_id is absent throws "Cannot read property 'remoteUpdate' of undefined". Added a second guard with a warning log.


8. cinnamonDBus: remove leftover debug comment in GetInputSources

Removed a stray // global.log(source.preferences); debug line accidentally left in GetInputSources().


Test plan

  • PCI device description cleanup in network manager shows all underscores/commas replaced and all parenthesised segments stripped
  • Keyboard layout switching works correctly (no regressions from var fix)
  • Notifications with non-standard urgency values display the information icon
  • No crash when a short-expiry notification fires before its source PID is resolved
  • Display-change countdown dialog shows correct plural form ("5 seconds", "1 second")
  • No crash in cinnamon-settings when calling callbacks or updating settings for a removed xlet
  • GetInputSources D-Bus method works correctly

🤖 Generated with Claude Code

KAMI911 and others added 8 commits March 14, 2026 12:39
The two regex replacements in fixupPCIDescription() were missing the /g
(global) flag, causing only the first occurrence of each pattern to be
replaced rather than all occurrences:

- /[_,]/ → /[_,]/g  (replace all underscores/commas with spaces)
- /\([\s\S][^\(\)]{2,}\)/ → same with /g  (strip all parenthesized info
  longer than 2 characters, not just the first one)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five schema/key constants at the top of the file were assigned without
a var/let/const keyword, making them implicit globals that pollute the
global scope. Inconsistent with the var declarations used elsewhere in
the same file.

Add var to each: DESKTOP_INPUT_SOURCES_SCHEMA, KEY_INPUT_SOURCES,
KEY_KEYBOARD_OPTIONS, KEY_PER_WINDOW, KEY_SOURCE_LAYOUTS.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The switch on hints.urgency had no default branch, leaving stockIcon
undefined when urgency is any value other than LOW (0), NORMAL (1), or
CRITICAL (2). Passing icon_name: undefined to St.Icon silently creates
a broken icon actor.

Fall back to the information icon for unrecognised urgency values,
matching the behaviour for LOW and NORMAL urgency.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n is null

When NotifyAsync receives a notification for a sender it has not seen
before, it adds ndata to _expireNotifications and starts the expire
timer before the async GetConnectionUnixProcessID D-Bus call completes.
ndata.notification is only set later, inside _notifyForSource(). If the
expire timer fires during that window, _expireNotification() called
ndata.notification.destroy() on undefined, crashing with a TypeError.

Guard the destroy call: if ndata.notification does not exist yet, remove
the orphaned entry from the queue and restart the timer for the next one.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dialog

ngettext(singular, plural) requires a third argument — the integer count
used to select the correct plural form. Without it the function always
returns the singular string ("Reverting in 5 second." instead of
"Reverting in 5 seconds.").

Pass this._countDown as the count, matching the pattern used in
endSessionDialog.js.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_getXletObject() returns null when no applet, desklet, or extension
matches the given uuid/instance_id combination. The very next line then
called null[callback].bind(null), throwing a TypeError.

This can happen when cinnamon-settings calls a callback for an xlet that
has been removed without a full shell restart. Add a null guard and log
a warning instead of crashing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
updateSetting() already guards against an unknown uuid, but not against
an unknown instance_id within a valid uuid. Accessing
uuids[uuid][instance_id].remoteUpdate() when instance_id is absent
throws "Cannot read property 'remoteUpdate' of undefined".

This can happen when cinnamon-settings updates a setting for an xlet
instance that was removed without a full shell restart. Add a second
guard and log a warning instead of crashing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove a stray '// global.log(source.preferences);' debug line that
was accidentally left in GetInputSources().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant