Uh oh!
There was an error while loading. Please reload this page.
Fix several bugs: regex flags, implicit globals, crashes, ngettext, undefined icon - #13643
Fix several bugs: regex flags, implicit globals, crashes, ngettext, undefined icon#13643KAMI911 wants to merge 10 commits into
Conversation
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>
| if (!Main.settingsManager.uuids[uuid][instance_id]) { | ||
| global.logWarning( | ||
| `[CinnamonDBus] [${uuid}] Unable to find instance '${instance_id}' from SettingsManager - ` + | ||
| 'this is likely due to configuring settings from a removed xlet instance.' |
There was a problem hiding this comment.
.... uuid] Unable to find instance 'instance_id' from SettingsManager' is enough, keep the message short.
mtwebster
commented
Mar 30, 2026
I'm ok with everything but expired notification changes - You're removing the ndata from the expire queue, but it's still (theoretically) in flight with It's all very unlikely in any event - this can only happen if a notification sets a custom timeout of under 1 second, and the dbus lookup takes longer than that (extremely unlikely for a local call like that). Considering the complexity handling this case would add, I would just leave this code alone, unless you have a real-world scenario where this occurs. The remainder of the PR looks fine (other than the overly-long log warning). |
…fication is null Per mtwebster's review: removing the pending entry from the expire queue can leave it orphaned once the async PID lookup completes and the notification is finally created, so it would never expire. The race this guarded against (a custom timeout under 1 second combined with a local D-Bus call taking longer than that) is negligible, so just drop the change and leave the original code as is.
Per review feedback: the message doesn't need the extra explanatory sentence, a single short line is enough.
KAMI911
commented
Aug 22, 2026
Thanks for the review! I've pushed two follow-up commits:
For reference, in case it's ever worth revisiting: instead of shifting the pending entry out of _expireNotification: function(){letndata=this._expireNotifications[0];if(ndata){if(ndata.notification){ndata.notification.destroy(MessageTray.NotificationDestroyedReason.EXPIRED);}else{// Notification actor not created yet (async PID lookup still// pending) -- keep it in the queue and retry shortly instead of// dropping it, so it still expires once _notifyForSource runs.this._expireTimer=Mainloop.timeout_add_seconds(1,Lang.bind(this,this._expireNotification));returnfalse;}}this._expireTimer=0;returnfalse;},That keeps the entry trackable so it still expires (and still emits |
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 replacementsTwo regex replacements in
fixupPCIDescription()were missing the/gflag, 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 first2.
keyboardManager: fix implicit global variable declarationsFive 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 withoutvar, making them implicit globals that pollute the global scope. Addedvarto match the style used elsewhere in the file.3.
notificationDaemon: add default case for unknown urgency in icon switchThe
switch (hints.urgency)in_iconForNotificationData()had nodefaultbranch, leavingstockIconasundefinedfor any urgency value outside LOW/NORMAL/CRITICAL. This silently created a brokenSt.Iconwithicon_name: undefined. Added adefaultcase falling back to the information icon.4.
notificationDaemon: fix crash in _expireNotification when notification is nullNotifyAsync()addsndatato_expireNotificationsand starts the expire timer before the asyncGetConnectionUnixProcessIDD-Bus call completes.ndata.notificationis only assigned later inside_notifyForSource(). If the timer fires during that window, callingndata.notification.destroy()onundefinedthrows aTypeError. 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 dialogngettext(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 inendSessionDialog.js.6.
cinnamonDBus: fix crash in activateCallback when xlet is not found_getXletObject()returnsnullwhen no applet/desklet/extension matches the given uuid/instance_id. The next line callednull[callback].bind(null), throwing aTypeError. 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 foundupdateSetting()already guarded against an unknownuuid, but not against an unknowninstance_idwithin a validuuid. Accessinguuids[uuid][instance_id].remoteUpdate()wheninstance_idis absent throws "Cannot read property 'remoteUpdate' of undefined". Added a second guard with a warning log.8.
cinnamonDBus: remove leftover debug comment in GetInputSourcesRemoved a stray
// global.log(source.preferences);debug line accidentally left inGetInputSources().Test plan
varfix)GetInputSourcesD-Bus method works correctly🤖 Generated with Claude Code