Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make notification states immutable #12260

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hduelme
Copy link
Contributor

@hduelme hduelme commented Jun 29, 2024

Ensuring that notification states are immutable helps prevent any unwanted or accidental modifications.

@rcalixte
Copy link
Member

Can you expand on the change here a little bit? What are the differences in the before/after in the user experience? Are there any examples that might be useful?

@hduelme
Copy link
Contributor Author

hduelme commented Jun 29, 2024

Currently anyone using the API (for example spices) could accidental modify the stats, for example MessageTray.Urgency.CRITICAL = 0; could accidental be executed, resulting in a wrong internal state for CRITICAL. This change prevents this. Any changes made to this would be ignored, as cinnamon don't run in strict mode. For the end-user nothing should change at all.

@mtwebster
Copy link
Member

Hi, rebased onto current Cinnamon, I get:

Gjs-Message: 09:57:04.741: JS LOG: Enabling WindowAttentionHandler
Gjs-Message: 09:57:08.669: JS LOG: [LookingGlass/error] Urgency is undefined
Gjs-Message: 09:57:08.669: JS LOG: [LookingGlass/trace] 
<----------------
update_list@/usr/share/cinnamon/applets/[email protected]/applet.js:179:1
_notification_added@/usr/share/cinnamon/applets/[email protected]/applet.js:161:14
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_hideNotificationCompleted@/usr/share/cinnamon/js/ui/messageTray.js:1113:18
_tweenComplete@/usr/share/cinnamon/js/ui/messageTray.js:948:24
_addHandler/params[name]@/usr/share/cinnamon/js/ui/tweener.js:256:24
_callOnFunction@resource:///org/gnome/gjs/modules/script/tweener/tweener.js:190:16
_updateTweenByIndex@resource:///org/gnome/gjs/modules/script/tweener/tweener.js:320:24
_updateTweens@resource:///org/gnome/gjs/modules/script/tweener/tweener.js:333:18
_onEnterFrame@resource:///org/gnome/gjs/modules/script/tweener/tweener.js:348:10
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_onNewFrame@/usr/share/cinnamon/js/ui/tweener.js:413:14
_init/<@/usr/share/cinnamon/js/ui/tweener.js:389:22
---------------->

@hduelme hduelme force-pushed the make-notification-states-immutable branch from ecda3f0 to f6ca1b7 Compare November 2, 2024 01:49
@hduelme
Copy link
Contributor Author

hduelme commented Nov 2, 2024

Sorry for my late response. I was able to reproduce the error. The problem occurs because I change var to const. The way the states are accessed requires them to be global scoped. I've reverted them to var to resolve the issue.

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.

3 participants