After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668214 - gnome-shell writes to GSettings on login
gnome-shell writes to GSettings on login
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: telepathy
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Shell Telepathy maintainer(s)
gnome-shell-maint
Depends on:
Blocks: 671566
 
 
Reported: 2012-01-18 20:45 UTC by Allison Karlitskaya (desrt)
Modified: 2012-03-07 15:49 UTC
See Also:
GNOME target: 3.4
GNOME version: ---


Attachments
userMenu: Only save session status when necessary (1.55 KB, patch)
2012-01-19 19:27 UTC, Florian Müllner
none Details | Review
userMenu: Only save session status when necessary (1.69 KB, patch)
2012-03-03 09:39 UTC, Florian Müllner
committed Details | Review

Description Allison Karlitskaya (desrt) 2012-01-18 20:45:03 UTC
applications should only write to GSettings as the result of an explicit user action.  gnome-shell is doing it on login.

The code in question is this:

        global.settings.set_int('saved-session-presence', sessionStatus);

in userMenu.js
Comment 1 Allison Karlitskaya (desrt) 2012-01-19 18:06:31 UTC
It's worth noting that with the closing of a similar bug in gnome-settings-daemon, this bug is now the only reason that dconf-service is started at all during a login.
Comment 2 Florian Müllner 2012-01-19 19:12:36 UTC
No worries, patch coming ...
Comment 3 Florian Müllner 2012-01-19 19:27:52 UTC
Created attachment 205657 [details] [review]
userMenu: Only save session status when necessary

Saving the status to GSettings is pointless if it did not actually
change. If done during login, it is actually harmful, as it causes
dconf-service to be started.
Comment 4 Allison Karlitskaya (desrt) 2012-01-19 19:54:59 UTC
Review of attachment 205657 [details] [review]:

::: js/ui/userMenu.js
@@ +399,3 @@
 
+        if (savedStatus != sessionStatus)
+            global.settings.set_int('saved-session-presence', sessionStatus);

This will solve the reported issue of causing GSettings to be activated, but I'm not sure it's the best fix possible.

In my opinion, we should only store in GSettings the value that the user explicitly set by interacting with the shell UI.  On login, we should drive the value to this setting.  That means that if I explicitly went online but my network connection dropped out, then at next login, the shell would still try to put me online by default (which I think is correct).

As it is now, if my connection drops, this setting gets changed and on next login I'm left logged out.

It also means that it's still possible that GSettings is written to without an explicit user action...
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-01-19 20:05:24 UTC
(In reply to comment #4)
> In my opinion, we should only store in GSettings the value that the user
> explicitly set by interacting with the shell UI.  On login, we should drive the
> value to this setting.  That means that if I explicitly went online but my
> network connection dropped out, then at next login, the shell would still try
> to put me online by default (which I think is correct).

Hypothetical case: if gnome-session put you at "busy" because you weren't at the machine, and an automatic reboot happened (scheduled), and you have set your machine to auto-login, should you be at "Online" or "Busy"? I'd say the latter.

Someone smarter than me can come up with a less crazy case why you'd want to have a automatically set status persist.
Comment 6 Allison Karlitskaya (desrt) 2012-01-19 20:15:37 UTC
I assume you mean 'Idle'.  And after an automatic reboot, I wouldn't expect the first thing in the session to be that I'm idle -- more likely the automatic reboot took me to the login screen and then I explicitly logged in.  I'm certainly not idle in that case.

If we wanted to deal with that case properly, it would have to involve asking the X server what the last user interaction was and dealing with it accordingly -- idle vs. not-idle is *definitely* not a preference/setting and should not be in GSettings.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-01-19 20:26:31 UTC
As I said before, the keys here are really persistent state, not settings. We just don't have a better place to store persistent state.
Comment 8 Allison Karlitskaya (desrt) 2012-01-19 20:29:49 UTC
If the session is idle or not certainly doesn't even fall under the banner of 'persistent' state.
Comment 9 Florian Müllner 2012-01-19 20:42:41 UTC
(In reply to comment #4)
> In my opinion, we should only store in GSettings the value that the user
> explicitly set by interacting with the shell UI.

I think I can agree with you for this setting, although it looks like you are mistaken about what is saved in that setting - in terms of shell UI, it is the status of the "Notifications" switch. As we don't really do session-saving, whatever caused the status to be changed outside the shell probably no longer applies at the next login, so I see a point in ignoring it.


> That means that if I explicitly went online but my
> network connection dropped out, then at next login, the shell would still try
> to put me online by default (which I think is correct).
> 
> As it is now, if my connection drops, this setting gets changed and on next
> login I'm left logged out.

As mentioned above, that's a different status, and I don't agree that it's that clear. There are basically two cases where the IM status is changed from outside the shell:

(1) the user changes his/her status in Empathy
(2) the network is disconnected

I believe that the current behavior is correct for (1), but incorrect for (2). So what we actually want in my opinion is network-awareness to differentiate the two cases - the telepathy folks mentioned that there'd be API for that at some point (or roll our own really, as we obviously depend on nm already).

That should really get its own bug report though.


> It also means that it's still possible that GSettings is written to without an
> explicit user action...

Correct. But what you suggest would mean that it's possible that GSettings are not written despite explicit user action ...
Comment 10 Milan Bouchet-Valat 2012-01-20 10:00:39 UTC
Less crazy case: you're online, network disconnects (e.g. no WiFi where you are). You turn off your machine for the night to apply updates or anything, and next morning, while you start your work, you suddenly realize you're online on IM. Quite unexpected, and there's no way to avoid being connected, at least for a few seconds.

The problem is complex, but I think it makes sense to always restore the state to what it was on logout. Actually, this debate is only about online/offline AFAICT, since other statuses are only set manually. Basically, what I'm suggesting is that IM status behaves just as if no reboot had happened.
Comment 11 Florian Müllner 2012-01-20 10:53:58 UTC
(In reply to comment #10)
> Actually, this debate is only about online/offline AFAICT, since other 
> statuses are only set manually.

Not true, we store every status that is not the result of an automatic change we did ourselves.
Comment 12 Milan Bouchet-Valat 2012-01-20 13:58:50 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Actually, this debate is only about online/offline AFAICT, since other 
> > statuses are only set manually.
> Not true, we store every status that is not the result of an automatic change
> we did ourselves.
What's what I meant. The only status that (if I'm not mistaken) can be set automatically by the Shell and would be interesting to remember accros reboots is "offline".
Comment 13 Florian Müllner 2012-01-20 14:18:14 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Actually, this debate is only about online/offline AFAICT, since other 
> > > statuses are only set manually.
> > Not true, we store every status that is not the result of an automatic change
> > we did ourselves.
> What's what I meant.

Ah, good. I just wanted to clarify that we do remember statuses set from Empathy (regardless of whether they are in the shell menu by default or not).


> The only status that (if I'm not mistaken) can be set
> automatically by the Shell and would be interesting to remember across reboots
> is "offline".

No, we don't set "offline" automatically. "Offline" is either the result of user action, or telepathy itself changing the status (usually because the network goes down) - we currently save the status in either case, but I agree with Ryan that we should not do so in the latter case.
Comment 14 Matthias Clasen 2012-03-02 21:48:45 UTC
See bug 665701 for the harm that is being done by setting the status to idle at login. This is obvious nonsense and we must not do it.
Comment 15 Florian Müllner 2012-03-03 09:39:48 UTC
Created attachment 208899 [details] [review]
userMenu: Only save session status when necessary

Rebased to master
Comment 16 Jasper St. Pierre (not reading bugmail) 2012-03-03 09:53:07 UTC
Review of attachment 208899 [details] [review]:

Makes sense.
Comment 17 Florian Müllner 2012-03-03 10:23:49 UTC
Attachment 208899 [details] pushed as 2b87051 - userMenu: Only save session status when necessary