Continue running when screen is locked and optionally show on lockscreen, using session modes #773
Conversation
This file contains 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
…lockscreen in an officially-supported manner See https://gjs.guide/extensions/topics/session-modes.html
…ackground Hidden by default. This makes use of the session modes feature introduced in GNOME Shell 42. https://gjs.guide/extensions/topics/session-modes.html Strangely, disconnecting the session mode updated callback, like in the example, resulted in the applet still being shown the first time the screen is locked. It seems to work fine with it left out, so I've just done that.
ZimbiX
deleted the
continue-running-when-screen-is-locked-and-optionally-show-on-lockscreen
branch
|
FYI the
From https://gjs.guide/extensions/review-guidelines/review-guidelines.html#session-modes:
Might be a bit of a challenge to convince the reviewer of the first point... but doesn't hurt to try. I'll put something together in the |
|
@mgalgs Thanks for the update. I'll keep this PR referenced in my notes in case I need to revert it. |
|
Damn. Sorry to have missed that! Looks like they're going to get hung up on 'Disconnect all signals' as well =\
|
I believe that's separate from the session mode requirements, but yeah, seems like that requirement is being violated as well, maybe just snuck through initial review... 😬
I've been playing around with this so that I can add the disconnect before re-submitting for review by the gnome team and have encountered some strange behavior. I've done very little actual GS extension development so maybe this is expected but it definitely seems strange... Would appreciate a second set of eyes. When I lock the screen the first time after enabling the extension I actually see a flurry of disables/enables in the logs... Is anyone else seeing this?? My logs are below. I have the disconnect wired up plus a few additional debug logs as per this diff: @@ -49,7 +49,7 @@ var Me = ExtensionUtils.getCurrentExtension();
var Convenience = Me.imports.convenience;
var Compat = Me.imports.compat;
-var Background, GTop, IconSize, Locale, MountsMonitor, NM, NetworkManager, Schema, StatusArea, Style, gc_timeout, menu_timeout;
+var Background, GTop, IconSize, Locale, MountsMonitor, NM, NetworkManager, Schema, StatusArea, Style, gc_timeout, menu_timeout, session_id;
try {
GTop = imports.gi.GTop;
@@ -2403,9 +2403,12 @@ function init() {
}
function _onSessionModeChanged(session) {
+ log(`sesh stuff: ${session.currentMode} ${session.parentMode} ${Schema.get_boolean('show-on-lockscreen')}`);
if (session.currentMode === 'user' || session.parentMode === 'user') {
+ log("Therefore showing");
Main.__sm.tray.show()
} else if (session.currentMode === 'unlock-dialog' && !Schema.get_boolean('show-on-lockscreen')) {
+ log("Therefore hiding");
Main.__sm.tray.hide()
}
}
@@ -2578,7 +2581,7 @@ function enable() {
Main.panel.menuManager.addMenu(tray.menu);
if (shell_Version >= '42') {
- Main.sessionMode.connect('updated', _onSessionModeChanged);
+ session_id = Main.sessionMode.connect('updated', _onSessionModeChanged);
}
}
log('[System monitor] applet enabling done');
@@ -2610,6 +2613,14 @@ function disable() {
Style = null;
}
+ // Some users would like to monitor system activity even when the
+ // screen is locked.
+ if (session_id) {
+ log(`bye ${session_id}`);
+ Main.sessionMode.disconnect(session_id);
+ session_id = null;
+ }
+
Schema.run_dispose();
for (let eltName in Main.__sm.elts) {
Main.__sm.elts[eltName].destroy();Here's my initial screen lock after enabling the extension. This is after a fresh login so overall state should be clean. Screen is now locked. Note that the session mode update signal never fired. The extension is still visible at this point, presumably due to the simple presence of Just look at all those session ids... 🤔 Anyway, here's the screen unlock. We finally see the session mode update signal: Now I lock the screen again (this is the second time since the extension was enabled), and this time we don't see all the enables/disables and we do see the session mode update signal: Extension is hidden as expected. And unlocking after the second lock looks good as well: |
|
And yeah, if I don't disconnect the session change signal handler in (diff from the previous) @@ -2617,7 +2617,7 @@ function disable() {
// screen is locked.
if (session_id) {
log(`bye ${session_id}`);
- Main.sessionMode.disconnect(session_id);
+ // Main.sessionMode.disconnect(session_id);
session_id = null;
}First lock: and the unlock: Second lock (no enable/disable spamming but the mode change handler is still spammed): and the unlock: |
|
Good work, @mgalgs. I appreciate the help with this. Despite it not being like this in the documentation, we could try moving the |
|
The docs actually forbid connecting signals in First thing I want to look at is dangling connected signals. Maybe if we just get everything disconnected properly the extension will unload the first time rather than gnome shell having to hammer on it? |
|
Hah, I really should RTFM 🤦 |
|
I haven't gotten a chance to debug this further. What do you want to do here, @chrisspen? I feel like we should probably revert this until someone gets to the bottom of the above issues. |
|
I haven't had the motivation to put any more time into this yet either. I'm ok with reverting for now to get a release out the door finally, but publishing the release might depend on @paradoxxxzero doing it himself (unclear from #767)? |
|
Reverting it will at least unblock |
|
Fair enough |
|
@chrisspen Please either:
|
|
We should at least merge the README changes from #780 |
|
Ah yes, sorry, I forgot about that |
mgalgs
added a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Apr 21, 2023
…phs in background" This reverts commit f251e72. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
mgalgs
added a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Apr 21, 2023
… on the lockscreen in an officially-supported manner" This reverts commit 0069a85. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
mgalgs-bot
pushed a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Apr 21, 2023
…phs in background" This reverts commit f251e72. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
mgalgs-bot
pushed a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Apr 21, 2023
… on the lockscreen in an officially-supported manner" This reverts commit 0069a85. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
mgalgs-bot
pushed a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Oct 19, 2023
…phs in background" This reverts commit f251e72. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
mgalgs-bot
pushed a commit
to mgalgs/gnome-shell-system-monitor-next-applet
that referenced
this pull request
Oct 19, 2023
… on the lockscreen in an officially-supported manner" This reverts commit 0069a85. Doesn't adhere to gnome's session mode guidelines. paradoxxxzero#773 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #329. Resolves #624. On Gnome Shell 42+, that is.
This change allows the extension to continue running while the screen is locked, in an officially-supported manner - by making use of the new session modes feature introduced in Gnome Shell 42. This is as opposed to the frowned-upon approach of disabling the
disable()function (as implemented by @franglais125), which would apparently prevent publishing to the Gnome Extensions website, and also mean the user can't turn off the extension without removing it.I've added an option to choose whether the applet should be shown on the lockscreen. When not enabled, the applet is simply hidden. And it's hidden on the lockscreen by default.
Strangely, disconnecting the session mode updated callback in
disable(), like in the example code, resulted in the applet still being shown the first time the screen is locked. I couldn't work out why that was happening. But it seems to work fine with it left out, so I've just done that.I developed this on Gnome 43.2, while branched from #754 by necessity; but it should work here on Gnome 42 as well. And hopefully it changes nothing on older versions of Gnome.
Screenshots (from my fork - there's a couple of other changes in there, but you get the gist):