Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds a native screensaver implementation to Cinnamon, migrating functionality from the external cinnamon-screensaver daemon into Cinnamon itself. The implementation reuses PAM authentication mechanisms and provides feature parity for both X11 and Wayland sessions.
Changes:
- Integrated PAM authentication with native unlock dialog and away message support
- Implemented screensaver widgets (clock, album art with MPRIS control, power/battery indicator)
- Added action modes system for keybinding context management during lock/unlock states
- Unified MPRIS and power utility modules for sharing between sound applet and screensaver
- Updated session files, packaging, and applets to use internal screensaver
Reviewed changes
Copilot reviewed 57 out of 59 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/screensaver/*.{c,h} | PAM authentication, setuid handling, subprocess management (imported from cinnamon-screensaver) |
| src/cinnamon-screensaver-pam-helper.c | PAM helper subprocess for authentication |
| js/ui/screenShield.js | Main screensaver coordinator with widget floating system and state management |
| js/ui/unlockDialog.js | Unlock dialog UI with password entry and user switching |
| js/ui/awayMessageDialog.js | Modal dialog for setting away message before locking |
| js/ui/screensaverWidgets/*.js | Floating widgets for clock, album art, and power display |
| js/misc/{authClient,loginManager,mprisPlayer,powerUtils}.js | Infrastructure modules for authentication, session management, and shared utilities |
| js/ui/main.js | Action mode system for keybinding filtering based on screen state |
| js/ui/keybindings.js | Action mode support for keybindings |
| files/usr/share/cinnamon/cinnamon-screensaver-command/*.py | Replacement screensaver command tool using DBus |
| data/pam/*.pam | PAM configuration files for authentication |
| debian/* | Packaging changes to provide cinnamon-screensaver and add PAM dependencies |
| *.session.in | Removed cinnamon-screensaver as required session component |
| data/theme/cinnamon-sass/widgets/_screensaver.scss | Styling for screensaver widgets and unlock dialog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| return FALSE; | ||
| } | ||
| return FALSE; |
There was a problem hiding this comment.
Unreachable code: this return statement will never be executed because all paths above either return TRUE or FALSE via goto out.
|
|
||
| // Look up the binding in our keybinding manager | ||
| let bindingName = binding.get_name(); | ||
| let [action_id, entry] = Main.keybindingManager._lookupEntry(bindingName); |
There was a problem hiding this comment.
Unused variable action_id.
files/usr/share/cinnamon/cinnamon-screensaver-command/cinnamon-screensaver-command.py
Show resolved
Hide resolved
files/usr/share/cinnamon/cinnamon-screensaver-command/cinnamon-screensaver-command.py
Show resolved
Hide resolved
Member
Author
|
/generate-test-packages |
|
Test packages generated successfully! Download from the workflow run (available for 7 days). |
mtwebster
force-pushed
the
screensaver
branch
from
e206310 to
0065487
Compare
mtwebster
force-pushed
the
screensaver
branch
3 times, most recently
from
10db390 to
9f0c514
Compare
mtwebster
force-pushed
the
screensaver
branch
from
9f0c514 to
ff4afda
Compare
callback. This lets us more easily control what keybindings are available in a given 'mode'. Assign modes to various states - expo, overview, etc...
- Re-uses PAM mechanisms from cinnamon-screensaver - Functional in wayland or x11 - Add a native 'away message' dialog - Unify 'show-greeter' code from multiple sources - Add systemd/consolekit support for session states - Provide cinnamon-screensaver-command (still utilized by csd- power for pre-power-event activation). - debian/control: Provide cinnamon-screensaver - Remove cinnamaon-screensaver as a required session component. - Unify mpris code for the sound applet and albumArtWidget.js - Unify power code for the power applet and powerWidget.js TODO: - Notification monitor (for feature parity with cinnamon-screensaver). - Wallpaper in wayland sessions - fprintd testing - debian/control fixes to allow replacement of cinnamon-screensaver during upgrade (cinnamon will now Provide cinnamon-screensaver). Imported from cinnamon-screensaver: - PAM-related files, cinnamon-screensaver-pam-helper - backup-locker and gdk-filter/event-grabber (simplified) Requires: linuxmint/muffin#797
Make the window internal instead.
the backup locker if cinnamon crashes. - Added a simple dbus interface for cinnamon-launcher. When the restart dialog is open, the interface can be used to respond 'yes' to the dialog, restarting cinnamon for us. - Add a button to the backup locker 'oops we crashed' view, to call for a restart. The old tty instructions are shown otherwise. - Use XCompositeOverlayWindow() to get a handle on the stage window. If cinnamon crashes, the transition to the backup locker instructions/button will be smooth (no desktop shown). It also helps during the restart, until Cinnamon once more takes over the compositor window. The COW trick is borrowed from muffin's restart-helper, which allows a smooth transition during controlled restarts (reexec_self).
The cinnamon-screensaver-command and cinnamon-unlock-desktop are provided by both cinnamon and cinnamon-screensaver, so this is needed.
Otherwise the key events get caught during activation, and the unlock dialog is immediately shown.
The way our events are handled, we get what we need via the stage event handler and login manager signals.
We should already be holding an inhibitor before receiving PrepareForSleep.
together once more.
before passwords, and don't lock the screen if PAM fails to init.
when PAM is going thru another round. Forward error messages from the pam helper.
attempt with a NULL response if a response was expected.
Watch it wiggle!
mtwebster
force-pushed
the
screensaver
branch
from
5c7223c to
c6fd6d7
Compare
We won't have 6.8 until the release. My cinnamon-screensaver PR bumps to 6.7.0 instead (a safe dev version). This will prevent co-installation issues during the remaining dev cycle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement a screensaver.
power for pre-power-event activation).
TODO:
Imported from cinnamon-screensaver:
Requires:
linuxmint/muffin#797
linuxmint/cinnamon-screensaver#491 (to test 'internal-screensaver-enabled')