Premature lock when launching guest session

[Gunnar Hjalmarsson]

Binary package hint: indicator-session

When you launch a guest session via the menu item provided by indicator-session, indicator-session locks the regular session before calling /usr/share/gdm/guest-session/guest-session-launch. Since also guest-session-launch includes code for locking, the first locking is redundant.

So why do I bother? The reason is that I have explored various possibilities to customize the guest session behavior, and one of the ideas is to let the regular user select the guest session language on-the-fly. But if I add such code to the top of guest-session-launch and call it via indicator-session, the screen gets locked instantly, and I need to enter the password to be able to see the list dialog box that is used to select language.

I believe that the activate_guest_session() function in session-service.c needs to be altered, so that its call for lock_if_possible() is removed. I have attached a diff that suggests just that change.

[Ted Gould (ted)]

I guess I disagree with your assessment. I think that indicator-session *should* lock the screen and guest-session should not as it doesn't do things like check the gconf key for whether the screen should be locked at all.

Now, that being said. I'd be happy with a patch that added a configurable dialog in indicator-session for selecting the language. I think that'd be pretty cool.

[Gunnar Hjalmarsson (gunnarhj)]

@Ted Gould
I'm just a newbie Ubuntu user who have some ideas of customizing the guest session feature.
http://ubuntuforums.org/showthread.php?t=1566078
Please be sure that I'm not prejudiced with respect to where things should be done. ;-)

As far as I understand, everybody does not use indicator-session, and the docs for gdm-guest-session simply suggests that you call /usr/share/gdm/guest-session/guest-session-launch to start a session. As long as you don't force the users to start guest sessions via indicator-session, the gdm-guest-session package seems to be the the right place for the locking code IMO.

The current code in /usr/share/gdm/guest-session/guest-session-launch for locking the screen is simple:
gnome-screensaver-command --lock || xscreensaver-command -lock || true

It appears to me as if improving that code would be an adequate step if, as you indicate, the locking code in indicator-session is better.

[Gunnar Hjalmarsson (gunnarhj)]

The suggestions in blueprint customize-guest-session (link under "Related blueprints") were committed recently. That fact emphasizes the sensible in approving the attached patch and the proposed merger.

[Martin Pitt (pitti)]

I agree, but that's not something we can change in gdm-guest-session, since at this point it's too late. Closing the guest session task, keeping the indicator-session task open.

[Gunnar Hjalmarsson (gunnarhj)]

@ Martin
Please note that the patch does not suggest any change in gdm-guest-session There were two reasons why I still stated that gdm-guest-session is affected by this issue and subscribed Ubuntu Desktop:

1. Have confimed that gdm-guest-session does not rely on the screen locking that currently happens in indicator-session. Is your "I agree" such a confirmation? :)

2. Consider Ted's mentioning of shortcomings in gdm-guest-session's lock code.

Too late for 10.10, but not for 11.04, right?

[Vish (vish)]

Is this the cause of Bug #599351 ?
Might be the reason i often notice the screenlock dialogue for a split second before switching to guest-session or another user.

[Gunnar Hjalmarsson (gunnarhj)]

@ Vish
Even if the two bugs have points in common, they deal with different issues. The concern of this bug is the guest session menu item only, and I propose that the screen locking code is removed from that particular menu option in indicator-session.

[Gunnar Hjalmarsson (gunnarhj)]

@Ted
A locked screen is now a condition for launching a guest session, since launchs without preceding screen locking would defeat the purpose of the feature.
https://launchpad.net/ubuntu/+source/gdm-guest-session/0.19

Consequently, checking whether the screen should be locked does not apply.

I hope you reconsider, Ted.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Ted, thanks for the chat the other day! I have proposed a '--no-lock' option in gdm-guest-session:
https://code.launchpad.net/~gunnarhj/ubuntu/natty/gdm-guest-session/nolock/+merge/50497

Please note that the --no-lock option should only be applied by individual users, and not used in any menu option that is shipped with Ubuntu, or else we might lead people to mistakenly use gdm-guest-session in an insecure manner.

[Sebastien Bacher (seb128)]

gdm-guest-session (0.22) natty; urgency=low

  * gdm/guest-session-launch:
    - Added a --no-lock option.

[Sebastien Bacher (seb128)]

should the indicatort bug be closed or is there still something to do there?

[Gunnar Hjalmarsson (gunnarhj)]

@Sebastien Bacher
During a talk at #ubuntu-devel on February 17, Ted mentioned that developers may want to launch guest sessions without locking the screen in order to get a clean test environment. The gdm-guest-session --no-lock option is a response to that need only. It does not resolve the indicator bug, which is just as valid as before.

Thought that Ted agreed to remove the indicator code for locking guest sessions, btw. Ted?

[Ted Gould (ted)]

Hi Gunnar,

I'm going to mark this fixed, and I realize that it's not the way that you thought it was going to be fixed, but I want to explain why this way is better. I think it specifically takes your use-case into account and in fact enriches the very interesting work that you're doing.

I'm now launching the guest-session with the --no-lock parameter in indicator-session. So this means that this bug is closed because only indicator-session is locking the screen. That's better because indicator-session takes a more global view of when the screen should be locked and looks at other desktop settings. We want to keep that.

I realize you're thinking that this restricts you from doing custom guest sessions. And that's partially true in that you can't use the "Guest Session" entry for that. But, what we've added is much richer. Also in this release is the ability to add to the session menu arbitrary desktop files. What this means is that you can add multiple enriched sessions like the ones that you've documented how to do on the forums. And you can do you it in packages and very maintainable ways. I think this is exciting, and I can't wait to see what you do with it.

What you need to do is put a file in /usr/share/indicators/session/applications/ that is a .desktop file. That file will be put in the menu with the menu entry being the "Name" key. When that entry is clicked on the "Exec" line will be run. This can be any script. Indicator session will not take care of locking when these commands are run. There can be any number that you'd like.

Thanks for your passionate appeal for this change, I hope this gives you the tools to do some really cool things with Ubuntu.

Ted

[Launchpad Janitor (janitor)]

This bug was fixed in the package indicator-session - 0.2.15-0ubuntu1

---------------
indicator-session (0.2.15-0ubuntu1) natty; urgency=low

  * New upstream release.
    ∘ Support adding arbitrary items to the end of the session
      menu. (LP: #727823)
    ∘ Call guest session with --no-lock to ensure there is no
      double locking (LP: #636693)
 -- Ted Gould <email address hidden> Thu, 10 Mar 2011 16:10:19 -0600

[Gunnar Hjalmarsson (gunnarhj)]

@Ted
The issue at hand is that launching a guest session via the main (indicator-session) menu item prevents certain kinds of customization. That's just as true now as it was when I reported the bug. You did not fix the issue in some other way than I expected; you didn't fix it at all. Introducing another way to add custom launchers does not address the issue.

I changed the status accordingly. I also changed the bug summary to better reflect the actual issue of this bug report.

The main menu item for launching guest sessions should not unnecessarily make customization more difficult IMO, which it unfortunately does at the moment. I we cannot agree, and you insist in closing the bug, you'd better do it by using a proper status such as "Won't Fix" or "Opinion". Please don't say it's fixed when it's not.

[Martin Pitt (pitti)]

For the record, I would also prefer that the indicator not locking the screen by itself. The documented interface to launching a guest session including screen locking is to simply call /usr/share/gdm/guest-session/guest-session-launch.

But oh well, seems we agree to disagree, so the "Opinion" status seems appropriate.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Ted,

I decided to make a last (promise) attempt; please see the new merge proposal.

It struck me that it's unnecessary to fight about whose arguments are the better, when it's possible to make both of us happy. :)

[Ted Gould (ted)]

Gunnar,

I think that the issue here is that we really want "Guest Session" to do the same thing everywhere regardless of how they've configured their computer. If you want it to do something else, and I think you have some really interesting ideas about what it can do, let's give it a new name so that users can understand the differences.

What I don't understand is why you're against adding additional menu items for new features.

Ted

[Gunnar Hjalmarsson (gunnarhj)]

@Ted
No, no, it seems like I have failed to explain the reasons for my suggestion.

I want to give users options; I want to make it easy to customize the guest sessions. What makes guest sessions different compared to other kinds of sessions is that you can't make use of the normal tools within a session for customizing the behavior. It has to be done before launch, since no changes within a guest session are saved til next time you start a guest session.

I already mentioned the tutorial I wrote. For the time being that tutorial serves as a response to various user requests for customization options; please see bug 667089. Some settings are better done on-the-fly when launching a guest session, and that kind of customization (the example in the tutorial is the session language) must be done before the screen is locked.

I'm not against adding additional menu items when feasible; that's what people currently are advised to do in the tutorial. But that requirement makes customization more difficult, and with the suggested code change in the new merge proposal, using the default "Guest Session" menu item would work just fine.

During our IRC talk you mentioned interesting examples of how gdm-guest-session may be used, that I hadn't thought of before. To make it easier to launch guest sessions for such purposes, I made your suggestion for a --no-lock option be implemented. Now I ask you to please make on-the-fly customization easier by approving the new merge proposal.

Gunnar

[Gunnar Hjalmarsson (gunnarhj)]

Think I didn't really answered the question you asked, Ted.

On 2011-03-15 20:36, Ted Gould wrote:
> What I don't understand is why you're against adding additional menu
> items for new features.

I see it as customization of an existing feature, i.e. the tool for launching restricted guest sessions. We are talking about giving the users the opportunity to relatively simply customize the sessions, and it seems not reasonable IMO that such customization would force them to create an additional launcher. You are not forced to do that with any other application you customize, right?

Please approve the MP. It makes it easier to customize sessions without taking away the opportunity to launch sessions without locking.

Gunnar

[Martin Pitt (pitti)]

Actually it strikes me that it should probably be the guest-session-launch script itself which should check the gconf key, and then lock or not. This would then be closest to how things should behave in an ideal world. But doing the gconf check from C is a lot faster, so Gunnar's branch looks great to me. It won't change the default behaviour, but will respect the user configuration.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Ted,
Would be good if this could be resolved in time for Natty.

Gunnar