A very similar requirement occurs for Sun Ray, but with the added twist, that this needs to be activated or not depending on the session/seat/display. (The feature to use a script-provided autologin user, which is being reintroduced, allows enabling/disabling autologin on a per-session basis.)

The behavior you describe could be implemented either as you suggest, by providing a means to use autologin with the option to show a greeter (with no prompt, but a 'login' button nevertheless).

Alternatively this could be implemented by not using autologin, but extending the PAM conversation callback facility by a PAM_PROMPT_NO_INPUT: a prompt that the user needs to acknowledge, but that does not otherwise require input. Such a facility would allow composing a PAM stack that needs no user input (*), but uses such a prompt to get the greeter shown. We might be able to use PAM_TEXT_INFO as this prompt type, but its semantics in a GUI context (wait for acknowledgement or not, show in popup dialog or inline in greeter, ...) are underspecified and there may be setups that rely on whatever happens to be current behavior.

(*) Such a pam stack could look like
   auth pam_set_user.so username=jack
   auth pam_prompt_no_input.so
(The PAM modules would need to be written, but are trivial)

The autologin+option approach may be more understandable to administrators, as this is still about automatically logging in a user, while the other approach has a different form of consistency: it would mean that either no greeter session is run or else greeter behavior is controlled by PAM.

The option approach requires a different way of running/controlling the greeter that has no auth activity associated and has no very obvious way to make this a case-by-case option. The PAM approach is intrinsically more dynamic and should require less changes within gdm proper.

Since there is no login dialog with the autologin mode, users could change the keyboard layout with gnome-keyboard-applet after you log into the GNOME session?
Actually GDM just sends $GDM_KEYBOARD_LAYOUT to gnome-settings-daemon.

Regarding to LANG, I'm back port the feature in bug 599273.
I also guess users could select languages since LiveCD could shows the language menu before the system boots?

The reason behind the need for keyboard/language Auto-popup during LiveCD login session is that generally LiveCD login a configured as Timed login, users will have to select the language/keyboard manually within this time. It's not an intuitive or accessible way. Autopopups will solve the issue...

Sunray requirement can also be bundled along with this, as it's the same core functionality which's activated when different conditions are met. I also think that the "Login" button is not needed for the AutoPopups, we should be auto-logged in once we select the language/keyboard settings, additional click on the "Login" button won't serve any purpose...

OK, I could understand this issue.
I thought your issue might be the installer/sunray instead of GDM yesterday but now I understood LiveCD is different from installerCD. there is no GUI between Grub boot and GDM autologin.

This is an interesting request however it might take some time.
FYI. bug 536387 is under the discussion.

Created attachment 148507 [details] [review]
gdm TimedLoginDelay=-1 patch

(In reply to comment #3)
> users will have to select the language/keyboard manually within this time. 

How about supporting TimedLoginDelay=-1 ?
-1 means infinite timeout.
The attachment enables it.

> 
> Sunray requirement can also be bundled along with this, as it's the same core
> functionality which's activated when different conditions are met. I also think
> that the "Login" button is not needed for the AutoPopups, we should be
> auto-logged in once we select the language/keyboard settings, additional click
> on the "Login" button won't serve any purpose...

However if the button is missed, there is no button to determine all user settings. Generally speaking, if I think a setting dialog, I think it would have [Apply] button?
On the other hand, if the additional language/keyboard popup is generated, I wonder how is the background. 
E.g. I think the session won't be started until the user language is selected.
If not, I think it would have the specified timeout.
Probably I think the background is drawn by GDM so I guess most of the requests is covered by the current TimedLogin mode?

Fujiwara-san,

The patch you have attached, as you mentioned covers some of the requirements, I would think however 2 more things are needed

1. The setting of timed logout delay to UINT_MAX,

login_window->priv->timed_login_delay = UINT_MAX;

should be controlled by another variable like

TimeLoginPopupGUI=True

2. When the above resource is set (along with other timed login variables), the language/keyboard should auto popup

I agree with your comments on the issue of allowing the user to log in without clicking on a login button... I think we can live with an additional click for LiveCD too.

I was working on a GUI popup patch, will see if I can combine yours with mine, will updte before end of the week on the progress.

On a second thought 
TimeLoginPopupGUI=True
does not seem right, as popping up of the language/keyboard layout inherently involves some time, setting values like

TimedLoginEnable=true
TimedLogin=<user id>
TimedLoginDelay=2
TimeLoginPopupGUI=True

Imply the language and keyboard selection popup is limited by 2 secs, which won't be right.

I think we will stick with the 

[daemon]
AutomaticLoginEnable=true
AutomaticLogin=<user_id>
AutomaticLoginShowGUI=true

variables and will mimic the behaviour of timed login with 
login_window->priv->timed_login_delay = UINT_MAX;
if the above Automatic* variables are set right.

Created attachment 149390 [details] [review]
patch with AutoLoginShowGUI with auto language/keyboard popups

This patch against gdm-2.29.1 seems to work, enable this by setting
in /etc/gdm/custom.conf

[daemon]
AutomaticLoginEnable=true
AutomaticLogin=<login-id>
AutomaticLoginShowGUI=true

Then keyboard/language will auto popup

May be this patch can be reduced further...

(In reply to comment #8)
> Created an attachment (id=149390) [details] [review]
> patch with AutoLoginShowGUI with auto language/keyboard popups

It looks fine with me. Probably it will get furthermore feedback from the maintainers.
Actually I'm not sure if my idea delay=-1 would be good here.

One thing to note that we are not directly setting the TimedLoginDelay=-1 in custom.conf, but if AutoLoginShowGUI=true then internally delay is set to -1, in turn setting login_window->priv->timed_login_delay = UINT_MAX

Created attachment 149462 [details] [review]
patch with AutoLoginShowGUI with auto language/keyboard popups, only popup languages when libxklavier is not present during compile time

This patch makes this work without libxklavier too

If you will update your patch next time,

+        active_item_id = gdm_option_widget_get_default_item (widget);

I think it needs a cast of GDM_OPTION_WIDGET in gdm_language_option_widget_popup_language_dialog() and gdm_layout_option_widget_popup_layout_dialog().
Also probably you don't need the cast of GDM_LANGUAGE_OPTION_WIDGET and GDM_LAYOUT_OPTION_WIDGET there.

Note comment #1.  It would be useful if this feature could be configured in a per-seat fashion.  

You can already specify that the automatic login user be defined in a per-display mannger by setting the username to "scriptname|".  The script is passed the DISPLAY variable so it can set the value based on display.  This is explained in the GDM 2.29 user docs in the TimedLogin and AutomaticLogin sections:

  http://library.gnome.org/admin/gdm/2.29/configuration.html.en#daemonconfig

So, it would be useful if this feature could also be configured in a per display fashion.  Perhaps the configuration option should also run a script which returns "1" for "yes, I want this display to show the GUI" and "0" for "no, I don't want this display to show the GUI".

Created attachment 149625 [details] [review]
patch with Sunray support

Hopefully this patch will serve the Sunray requirement, 
AutomaticLoginShowGUI
taken in a script and can popup the keyboard/language depending on the script output values.

I don't have a Sunray environment, need to check if this works on one. Will attach the test cases I tested...

Created attachment 149626 [details]
test cases for the patch

> Hopefully this patch will serve the Sunray requirement, 
> AutomaticLoginShowGUI
> taken in a script and can popup the keyboard/language depending on the script
> output values.
I meant "takes in a script..."

I just tested your patch applied to GDM 2.29.1.  Here are some comments:

1)  I understand that you want the language, layout dialogs to display so that
    the user is stepped through the process of selecting the keyboard layout and
    language.  However, I think this is awkward and does not work very well.

    GDM remembers the user's last language/layout/session choices in the user's
    $HOME/.dmrc file so the combo boxes in the panel should be pre-loaded with
    the user's last selected choices.  So, aside from the first time when a user
    sets the values, a user should never have to select the keyboard layout
    or language choices again.  Forcing the user to select the language & layout
    choices each time they login seems so cumbersome.

    It seems it would be better to not display the dialogs and just let the
    user pick the settings in the panel combo-boxes when they want to change 
    them.  This way, in the normal use-case, the user just needs to push the
    "Log In" button to login.  If this is acceptable, it makes the patch 
    considerably more simple since the code for popping up the dialogs would
    go away.

    I understand that in a LiveCD setting, it might be a bit nicer to force
    the dialogs to display since in this case you know the user does not have
    real settings in their $HOME/.dmrc file.  But, making GDM work this way 
    means that this feature does not work well in any other use case.

    Perhaps a compromise would be that GDM should only pop-up the dialogs 
    automatically if the user does not have a $HOME/.dmrc file and if 
    AutomaticLoginShowGUI is true?
   
2)  I can not seem to find "English (USA)" in the Language dialog.  To use 
    English do I need to pick some other country?  In other words, how do I
    select "just use the system default"?  I believe this is probably a 
    Solaris specific issue since I know that the locale code on Solaris works
    differently than on other distros.

3)  It seems odd to me to force the user to navigate the keyboard layout and
    language dialogs, but not to present a dialog for selecting the session.
    Why does it make sense to force the user to select only some things?

4)  After navigating the dialogs, the Login GUI has two choices "Automatic
    Login" and "Other".  I think this is a little confusing since some users
    might not understand what "Automatic Login" means.  It seems it would be
    better to display the actual user that is specified via the AutomaticLogin 
    key and "Other" instead.
   
5)  In the proposed patch you have this line:

    +#define GDM_KEY_AUTO_LOGIN_GUI "daemon/AutomaticLoginShowGUI"

    The variable should be the same as the key, so it should be
    GDM_KEY_AUTOMATIC_LOGIN_SHOW_GUI

    That helps keep things clear, so people don't get confused about what
    #define is associated with each key.

6)  I think the following code should be moved into the 
    gdm_slave_parse_piped_item rather than having similar code in both
    gdm_slave_parse_login and gdm_slave_parse_delay:

+        if (username_length > 0 && username[username_length-1] == '|') {
+                g_warning ("%s needs further parsing...", username);
+                return gdm_slave_parse_piped_item (slave, username, 
                                                    display_name);
         } else {

7)  The patch should only modify gdm-2.29.1/data/gdm.schemas.in.in and
    not gdm.schemas.in or gdm.schemas.  The last two files are generated and
    should not be a part of the patch.

8)  You need to provide docs for the new options (the new configuration key
    and the new behavior relating to how TimedLoginDelay=-1 works.  Please
    update docs/C/gdm.xml.  Here is a reference to the part of the docs that
    should be updated:

    http://library.gnome.org/admin/gdm/2.29/configuration.html.en#daemonsection

    The patch should include such documentation updates to get accepted 
    upstream.

9)  Note the coding style in the "HACKING" file in the gdm module.  Tabs are
    not allowed.  Your patch seems to have a lot of indentation issues where
    your changes do not line up with the surrounding code.  This should be
    corrected.

10) Please don't refer to the feature of supporting "show-gui" per-display as a
    "Sun Ray" feature.  I think this feature could be generally useful in a
    per-display fashion, and it isn't a Sun Ray specific feature at all.

Created attachment 150691 [details] [review]
Patch against gdm-2.29.4

Redone the patch to accommodate Brian's comments. Replies inlined.


(In reply to comment #17)
> I just tested your patch applied to GDM 2.29.1.  Here are some comments:
> 
> 1)  I understand that you want the language, layout dialogs to display so that
>     the user is stepped through the process of selecting the keyboard layout
> and
>     language.  However, I think this is awkward and does not work very well.
> 
>     GDM remembers the user's last language/layout/session choices in the user's
>     $HOME/.dmrc file so the combo boxes in the panel should be pre-loaded with
>     the user's last selected choices.  So, aside from the first time when a
> user
>     sets the values, a user should never have to select the keyboard layout
>     or language choices again.  Forcing the user to select the language &
> layout
>     choices each time they login seems so cumbersome.
> 
>     It seems it would be better to not display the dialogs and just let the
>     user pick the settings in the panel combo-boxes when they want to change 
>     them.  This way, in the normal use-case, the user just needs to push the
>     "Log In" button to login.  If this is acceptable, it makes the patch 
>     considerably more simple since the code for popping up the dialogs would
>     go away.
> 
>     I understand that in a LiveCD setting, it might be a bit nicer to force
>     the dialogs to display since in this case you know the user does not have
>     real settings in their $HOME/.dmrc file.  But, making GDM work this way 
>     means that this feature does not work well in any other use case.
> 
>     Perhaps a compromise would be that GDM should only pop-up the dialogs 
>     automatically if the user does not have a $HOME/.dmrc file and if 
>     AutomaticLoginShowGUI is true?
This is done in the patch.
> 
> 2)  I can not seem to find "English (USA)" in the Language dialog.  To use 
>     English do I need to pick some other country?  In other words, how do I
>     select "just use the system default"?  I believe this is probably a 
>     Solaris specific issue since I know that the locale code on Solaris works
>     differently than on other distros.
By default the popup shows the locale gdm is brought up in, unless the user explicitly sets otherwise this will be system default locale. 
> 
> 3)  It seems odd to me to force the user to navigate the keyboard layout and
>     language dialogs, but not to present a dialog for selecting the session.
>     Why does it make sense to force the user to select only some things?
Because there is no popup dialog for the session selection, but only an option menu. Also we have the gnome session by default, which will be sufficient for all normal users.
> 
> 4)  After navigating the dialogs, the Login GUI has two choices "Automatic
>     Login" and "Other".  I think this is a little confusing since some users
>     might not understand what "Automatic Login" means.  It seems it would be
>     better to display the actual user that is specified via the AutomaticLogin 
>     key and "Other" instead.
I have removed the "Select language and click Log In" message from appearing in the login screen in AutomaticLoginShowGUI mode. I think this enhancement if needed should be done as part of the AutomaticLogin enhancement and does not go very well if applied only to  AutomaticLoginShowGUI sessions. Also by default the Login names should not appear (unless a gconf variable is explicitly set) and forcing this to display will be a security issue.
> 
> 5)  In the proposed patch you have this line:
> 
>     +#define GDM_KEY_AUTO_LOGIN_GUI "daemon/AutomaticLoginShowGUI"
> 
>     The variable should be the same as the key, so it should be
>     GDM_KEY_AUTOMATIC_LOGIN_SHOW_GUI
> 
>     That helps keep things clear, so people don't get confused about what
>     #define is associated with each key.
Changed in patch
> 
> 6)  I think the following code should be moved into the 
>     gdm_slave_parse_piped_item rather than having similar code in both
>     gdm_slave_parse_login and gdm_slave_parse_delay:
> 
> +        if (username_length > 0 && username[username_length-1] == '|') {
> +                g_warning ("%s needs further parsing...", username);
> +                return gdm_slave_parse_piped_item (slave, username, 
>                                                     display_name);
>          } else {
> 
Changed in patch
> 7)  The patch should only modify gdm-2.29.1/data/gdm.schemas.in.in and
>     not gdm.schemas.in or gdm.schemas.  The last two files are generated and
>     should not be a part of the patch.
Removed from patch
> 
> 8)  You need to provide docs for the new options (the new configuration key
>     and the new behavior relating to how TimedLoginDelay=-1 works.  Please
>     update docs/C/gdm.xml.  Here is a reference to the part of the docs that
>     should be updated:
> 
>     http://library.gnome.org/admin/gdm/2.29/configuration.html.en#daemonsection
> 
>     The patch should include such documentation updates to get accepted 
>     upstream.
Provided the documentation
> 
> 9)  Note the coding style in the "HACKING" file in the gdm module.  Tabs are
>     not allowed.  Your patch seems to have a lot of indentation issues where
>     your changes do not line up with the surrounding code.  This should be
>     corrected.
Tabs removed from the patch
> 
> 10) Please don't refer to the feature of supporting "show-gui" per-display as a
>     "Sun Ray" feature.  I think this feature could be generally useful in a
>     per-display fashion, and it isn't a Sun Ray specific feature at all.
Noted.

What I need to do for getting this in to trunk ? (Waiting longer means patch may need to be redone to accommodate later changes in the code...)

I appreciate your patience.  I'd really like for Ray to review this patch and get his thoughts before moving forward.  Ray?

Brian and I had a long conversation about this a while ago. The conversation is below. I still haven't had a chance to look at attachment 150691 [details] [review] but I want to post the conversation since i've been so slow about tending to this bug report.

   [14:14:04]  <halfline> yippi_: bug 602663
   [14:14:14]  <halfline> i assumed you were pingng me about it :-)
   [14:14:51]  <yippi_> yes, it would be good to get some feedback about that
   patch for bug 602663.  It has been sitting for a long time.  It does add
   some interesting new features.
   [14:15:04]  <yippi_> Though there are a few things about that patch that I
   am unsure about.
   [14:15:34]  <halfline> i haven't looked at it yet
   [14:15:45]  <yippi_> First, the submitter really likes the idea of
   autopopping up the dialogs, which I'm not sure I think is necessary, and
   may undesirable.  Users can change the options without forcing them
   to navigate the dialogs.
   [14:16:03]  <yippi_> Though I believe the submitter fixed the patch so it
   only auto-pops up the dialogs if the user has no .dmrc file, so perhaps
   that isn't so bad.
   [14:16:40]  <yippi_> Also, this patch adds another question mark.  It has
   the need to support the feature per-display, and the new GDM doesn't
   really support per-display configuration anymore.
   [14:17:00]  <yippi_> I wonder if we should add back per-display
   configuration like the old GDM had, or whether we should manage such
   things via scripts.
   [14:17:36]  <yippi_> The current patch uses the script approach.  This is
   used already by the configuration variable to set the automatic login
   user.
   [14:18:39]  <halfline> not sure, honestly, i'd have to give the patch more
   of a look over than i'm prepared to right now to be able to weigh in
   [14:18:39]  <yippi_> the main desire with this bug is that it is useful to
   be able to pop-up the login screen to allow the user to set their
   language, keyboard, and session when autologin is being used.
   [14:18:54]  <halfline> i guess i don't understand
   [14:19:00]  <halfline> why timed login isn't good enough?
   [14:19:02]  <yippi_> so, in this case, the login screen displays but when
   the user clicks on the login button, they just login.
   [14:19:07]  <halfline> autologin was supposed to mean "no gui"
   [14:19:22]  <halfline> and timed login was supposed to mean "show gui so
   you can choose your language"
   [14:20:24]  <halfline> The thing i don't get, is how AutomaticLoginShowGUI
   is different than timed login
   [14:21:11]  <halfline> i guess the difference is the lack of a timeout?
   [14:21:45]  <yippi_> yes, perhaps we could better fix the issue by
   encouraging the submitter to use timed login.
   [14:22:07]  <yippi_> perhaps timedlogin could be fixed to support a
   special value so it has no timeout.
   [14:22:24]  <halfline> i mean clearly the submitter knows about timed
   login since he mentions it
   [14:22:37]  <halfline> so i guess i'm missing why it's not good enough for
   him
   [14:23:16]  <halfline> sounds like his patch just makes the option
   [14:23:24]  <halfline> do timed login delay=-1 under the hood
   [14:24:56]  <yippi_> i think the issue with timedlogin is that with a
   liveCD, there is only one user on the system.
   [14:25:11]  <yippi_> so, i think there is a desire to avoid presenting the
   user with the ability to select a different user.
   [14:25:45]  <yippi_> since that might be confusing.  AutoLogin means "i
   want to login as this particular user for sure" while timed login means "I
   will log into this user if you don't tell me a different user within a
   timeout"
   [14:26:38]  <yippi_> so, the desire is to be able to somehow tell GDM "I
   really want to login only as this user, I don't want to present the user
   with anything confusing about letting them think they can log in as a
   different user, but I also want the GUI to display so the user can specify
   their language/session/keyboard options"
   [14:27:00]  <halfline> so the thing is, one of the main reasons we added
   timed login back was for the livecd case
   [14:27:02]  <yippi_> which seems a bit in-between the current options.
   [14:27:16]  <halfline> i agree though it's not optimal
   [14:27:37]  <yippi_> but, perhaps the submitter of this patch is taking
   the wrong angle.  perhaps a better way to address this would be to add a
   timedlogin configure option to make it not ask for a different user
   [14:28:04]  <halfline> well i'm wonder if we took the wrong angle when we
   added timed login back
   [14:28:07]  <halfline> *wondering
   [14:28:17]  <yippi_> what do you mean?
   [14:29:12]  <halfline> i'm just saying it feels sort of arbitrary the way
   we're smack on features here
   [14:29:26]  <halfline> very organic, instead of thought through
   [14:29:39]  <yippi_> i have the suspicion that this patch is a bit of a
   prototype that just shows that it is possible to make GDM act as needed. 
   It wouldn't surprise me if we, after thinking about it, decide that to
   support this sort of feature should be configured differently than the
   patch proposes.
   [14:30:01]  <halfline> i'm saying maybe we need to back up and look at the
   bigger picture
   [14:30:15]  <halfline> what problem he wants solved, and what problems the
   features we already have solve
   [14:30:18]  <halfline> where there's overlap
   [14:30:38]  <halfline> and maybe consider redoing what we already have to
   better fit his needs
   [14:30:57]  <yippi_> there is a lot of overlap, that's true.  The problem
   is that this use-case sort of wants features from both autologin and
   timedlogin.
   [14:31:21]  <halfline> right, forget about timed login, forget about
   autologin
   [14:31:28]  <yippi_> i'm sure that would be acceptable.
   [14:31:39]  <halfline> let's think about this guys problem for a second
   [14:31:54]  <halfline> he wants to put his livecd in
   [14:31:55]  <halfline> boot up
   [14:32:08]  <halfline> let the user pick a language and then go to a
   desktop
   [14:32:18]  <halfline> he doesn't care about user selection
   [14:32:21]  <yippi_> or keyboard or session, obviously (if possible)
   [14:32:38]  <halfline> i wonder if his live cd supports multiple sessions?
   [14:32:50]  <yippi_> you mean via VT?
   [14:32:51]  <halfline> that's a little strange
   [14:32:52]  <halfline> no
   [14:32:57]  <halfline> i mean kde versus gnome
   [14:33:03]  <halfline> normally a livecd would only offer one
   [14:33:06]  <yippi_> actually i know it doesn't.
   [14:33:10]  <halfline> since livecds are small
   [14:33:17]  <yippi_> but it could be possible to offer multiple sessions. 
   [14:33:30]  <halfline> well sure, anything's possible to offer
   [14:33:41]  <halfline> trying to figure out what we *should* offer for his
   use case
   [14:33:54]  <yippi_> agreed, I think language is the most important. 
   However, if we are going to make it possible for the GUI to display, why
   not allow the user to change any of the reasonable settings.
   [14:33:56]  <halfline> we may need to offer other things too, but i'm just
   talking about his use case rght now
   [14:34:05]  <yippi_> if there is only one session, the session selector
   doesn't show up anyway
   [14:34:12]  <halfline> right, that's a good point
   [14:34:33]  <halfline> if there is a multi-session live cd we should show
   a session selector
   [14:34:42]  <halfline> so as long as we have the "hide if only 1" logic,
   we're fine
   [14:35:07]  <yippi_> in short, it seems we basically want the GDM GUI to
   show up to allow such selection, but to understand that we want to lock
   the user
   [14:35:11]  <yippi_> to a paritcular choice
   [14:35:36]  <halfline> we definitely want some GUI to show up
   [14:35:42]  <halfline> but if the primary point of the gui
   [14:35:43]  <yippi_> we could do this, I'd think, by adding a timedlogin
   configuration option to just disable the user selection bits
   [14:35:47]  <halfline> is to let the user pick a language
   [14:35:56]  <halfline> then do we want to hide the language down in the
   bottom of the screen
   [14:36:04]  <halfline> and have a window in the middle that the user can't
   do anything with?
   [14:36:16]  <yippi_> that's the issue that the user who submitted the
   patch has.  He wants to deal with this by auto-popping up the dialogs.
   [14:36:33]  <yippi_> which seems a reasonable choice if we want to make it
   more obvious that users need to do this.
   [14:36:47]  <halfline> it might be a better way to go
   [14:36:52]  <halfline> i haven't seen his patch yet
   [14:37:00]  <halfline> but if the only reason we're showing a UI
   [14:37:03]  <halfline> is for the user to pick language
   [14:37:10]  <halfline> it seems like the language should be front and
   center
   [14:37:20]  <yippi_> actually the patch auto-pops up both the language and
   keyboard dialogs
   [14:37:24]  <yippi_> one after the other
   [14:37:32]  <halfline> so that's probably suboptimal
   [14:37:36]  <yippi_> so i guess picking keyboard is also important to the
   submitter
   [14:37:41]  <halfline> maybe we could have a "first login" assistant
   [14:38:11]  <halfline> a wizard that pops up on screen before we start the
   users session
   [14:38:18]  <yippi_> yes, that would also solve the problem.  Perhaps
   autologin could detect if the user hasn't logged in before and start a
   separate wizard
   [14:38:42]  <halfline> it wouldn't even have to be autologin specific
   [14:38:51]  <halfline> the first time a user logs in period maybe
   [14:39:18]  <yippi_> actually it shouldn't be.  As mentioned in the bug
   report, the Sun Ray guys would like to have this feature as well for use
   in situations where it is not first-time-login
   [14:39:47]  <yippi_> they would like the ability to specify on-the-fly
   when it should be displayed.
   [14:40:29]  <halfline> is specifying defined as "rm -f ~/.dmrc" an
   acceptable definition for them?
   [14:40:54]  <yippi_> not as long as the touching of the user files can
   happen after pam-setcred
   [14:41:08]  <halfline> well what i'm asking is
   [14:41:22]  <halfline> can we, say, just show the wizard if .dmrc isn't
   around?
   [14:41:24]  <yippi_> though I'd guess a wizard could run after pam
   [14:41:30]  <halfline> it would have to run after pam
   [14:41:32]  <halfline> well
   [14:41:35]  <halfline> after pam_setcred
   [14:41:38]  <halfline> before pam_open_session
   [14:41:45]  <yippi_> right
   [14:41:52]  <yippi_> yes, that seems a good solution.
   [14:42:05]  <yippi_> perhaps we just pop up the dialogs without showing
   the rest of the GDM screen.
   [14:42:15]  <yippi_> and build the .dmrc file for the user.
   [14:42:25]  <halfline> so in the automatic login case
   [14:42:26]  <halfline> right now
   [14:42:36]  <halfline> we don't show the gui at all
   [14:42:53]  <yippi_> correct, and we shouldn't ever do so, unless we are
   in this wizard case
   [14:42:55]  <halfline> so we would just continue not showing the gui
   [14:43:23]  <halfline> actually
   [14:43:35]  <halfline> if we can show the wizard post pam_open_session
   [14:43:40]  <halfline> then this may be easier
   [14:44:06]  <halfline> basically
   [14:44:13]  <halfline> this would be a phase after the greeter is torn
   down
   [14:44:30]  <halfline> before starting the users session
   [14:44:34]  <halfline> or in the automatic login case
   [14:44:39]  <yippi_> when is PostLogin and PreSession called in the pam
   conversation?
   [14:44:43]  <halfline> we'd never have the greeter phase at all
   [14:44:59]  <yippi_> since if we want the deletion of .dmrc to be the way
   to say "i want the wizard", we need to make sure it happens after one of
   the hooks
   [14:45:45]  <yippi_> PreSession runs in on_session_started
   [14:45:47]  <halfline> post login is right after stopping the greeter
   [14:46:03]  <halfline> and presession is right before starting the sessoin
   [14:46:10]  <halfline> so we'd probably do the wizzard between those two
   hooks
   [14:46:15]  <halfline> brb
   [14:46:18]  <yippi_> right
   [14:49:08]  <yippi_> according to the GDM docs, it says that PostLogin is
   run before any session setup has been done, including before the
   pam_open_session call.  Is this still true?
   [14:50:40]  <halfline> nope
   [14:50:48]  <halfline> looking at the code that doesn't look true
   [14:51:26]  <halfline> we may need to fix that to reflect the docs
   [14:51:50]  <yippi_> Or fix the code, I suppose.  having a PostLogin hook
   after pam_open_session sort of defeats the purpose of having a scripting
   point there
   [14:52:01]  <yippi_> since it isn't really any different than PreSession
   then.
   [14:52:02]  <halfline> right i just said that
   [14:52:08]  <halfline> fix the code to reflect the docs
   [14:52:16]  <yippi_> oh, right
   [14:53:32]  <yippi_> does it matter for the bug we are reviewing, though?
   [14:53:43]  <halfline> nope, seems like an orthogonal issue
   [14:54:05]  <halfline> probably one that we don't need to prioritize very
   highly since no one has noticed or reported
   [14:54:50]  <yippi_> so let's ignore PostLogin then
   [14:55:13]  <yippi_> is the PreSession hook a suitable point to add the
   ability to remove the .dmrc file to indicate the wizard should start?
   [14:55:16]  <halfline> is it's not clear if they're for "us the developers
   of gdm"
   [14:56:17]  <halfline> yippi_: could be fine
   [14:56:27]  <halfline> yippi_: we'd have to run the wizard from
   on_session_started then
   [14:58:52]  <yippi_> we'd need to do the work before the worker calls
   do_start_session since that's when it caches or otherwise builds the .dmrc
   file
   [14:58:52]  <yippi_> i mean call the wizard when i say "do the work"
   [15:01:10]  <halfline> hmm
   [15:01:25]  <halfline> so there's a bit of an orderng problem there
   [15:01:47]  <halfline> do_start_session is what calls "SessionStarted"
   [15:02:03]  <halfline> which is what calls on_session_started we were
   talking about earlier
   [15:02:10]  <yippi_> so, it sounds like we have a reasonable approach
   towards solving the problem a better way.  do you want to update the bug
   or would you like me to?
   [15:02:12]  <halfline> which is what runs PreSession
   [15:02:43]  <halfline> well before i chime back in on the bug i want to
   actually look at his patch
   [15:02:45]  <halfline> and try it out
   [15:02:57]  <halfline> if you want to give an update before then, feel
   free
   [15:03:20]  <halfline> i think we sort of see a better direction to go
   [15:03:36]  <halfline> but i still want to fully try out his stuff
   [15:03:51]  <halfline> i'm just in the middle of another patch at the
   moment
   [15:04:17]  <yippi_> understood.  then i'll let you update the bug after
   you look at it.  doesn't make sense for me to say what to do, and then
   have you come to a different conclusion after trying it
   [15:04:45]  <yippi_> about the ordering issue, we'll need to highlight
   that problem.   but no reason the code couldn't be reorganized a bit to do
   the caching after the wizard
   [15:04:59]  <yippi_> that may just mean the caching happens after
   PreSession instead of before.
   [15:05:01]  <halfline> right
   [15:05:19]  <halfline> alright sounds good
   [15:05:29]  <halfline> i'm going to grab a quick snack and then burrow
   back into this patch

Thanks for the comments. The reason auto popup of language/keyboard needed was due to the following.

1. OpenSolaris LiveCD contains keyboard/language selection in the text console screen which appears before gdm with a numbered menu from which user can select the needed language/keyboard.

2. The above selection limits the choices to a screenful as there is no scroll capability for in the text console

3. Ubuntu provides a scrollable selection screens by providing customised extensions to GRUB, since OpenSolaris has moved on to GRUB2 seemed like these extensions are not supported.

4. Since newer gdm allows richer keyboard/language selection, we thought of removing the text console based limited selection and replicate the same functionality in gdm. Thats why this whole effort had began. Explicitly popping up keyboard/language is important (because essentially the console text screen selection of language/keyboards functionality is replaced with auto popups in gdm)

If AutomaticLoginEnable=true and AutomaticLogin has a login name defined, under the hood TImedLogin delay is set to -1 when AutomaticLoginShowGUI is parsed to be true/yes and if .dmrc file is not existing under the user $HOME. Difference between timed login and AutomaticLoginShowGUI mode is, since we are popping up the dialogues and let the user select them we cannot guarantee login within the fixed time. Also we would like to give the user a chance to correct the language/keyboard entries, if an error is made in selecting them. So the user need to press Login button before login (in case of TimedLogin after the timer expires, user automatically gets logged in)

The current functionallity is reached after a series of changes/rewrites. IMO this is not perfect, but good enough for the task at hand. I am not sure if I'll have the cycles for a further rewrite of this as a standalone 'First Time Login Assistant' utility. But I'll be curious to know what exactly is wrong with the current approach, and can spend time on rectifying that if needed.

I think one advantage of having a first-login helper is that the feature of helping the user set up their settings on first-time login is a general problem, while the proposed solution is designed only to work with Automatic Login.  It would be more useful to present such a settings helper to all users when they do not have a $HOME/.dmrc file.

Also, adding a new C-program to gui/simple-greeter that just pops up these widgets and sets the appropriate values should not be a significant amount of work.

The patch as it exists may be sufficient for your particular distro, and OpenSolaris can choose to add this patch regardless of whether the code goes upstream.  However, features that are added via a patch have a tendency to require maintenance and fixes as the underlying code changes.  It may be more work now to get the patch upstream, but it will be less work maintaining the code later.  If you choose to integrate the patch only in your distro, then understand that you will likely need to do ongoing work to keep the patch working as the underlying code changes.

Reg. the first-time login helper, does this need to be an application which lists the list of users in the first screen, list of sessions on next, followed by keyboards/languages. Selecting all of these will update the ~/.dmrc file for the user. 

Or can it be an application which can take in session, usernames in the command line and followed by keyboards/languages popups depending on which information ~/.dmrc is updated ?

I would think that this would be a simple GUI program which would only pop-up dialogs to ask the user to enter their session, keyboard, and language settings.
I would think that it could re-use the widget dialog code in GDM already for handling keyboard and language.  I wouldn't think this would be a command-line program.

There is no need for this program to request the username.  This program would be launched after authentication so the user would have either:

1) Already entered their username and password in the normal GDM dialog
2) If automatic/timed login is turned on, then the username would have been set
   in this manner already.

In other words, this would just be a simple program that would be launched after GDM has authenticated and before it starts the user session which checks if the user has a $HOME/.dmrc file.  If not, then it launches a dialog which says something like "This is the first time you have logged in, please select your preferences:" and pops up dialogs requesting the user enter their username, language, and keyboard.  Then it could set up the $HOME/.dmrc file for the user so these choices take effect.

Created attachment 159639 [details] [review]
Standalong utility for saving user prefs to .dmrc

Created attachment 159640 [details] [review]
 Standalone utility for saving user prefs to .dmrc

tabs removed from gdm-first-time-login-helper.c

This looks really good.  Some comments:

1) The helper is not integrated into the GDM code.  If you build GDM with this
   patch you can run "/usr/lib/gdm-first-time-login-helper (username)" by hand
   to test it, but it should be integrated into GDM so that it launches the
   program after PAM has finished if the user doesn't have a $HOME/.dmrc file.
   And it should obviously set the settings so they take effect when the user
   logs in.

   Also note in daemon/gdm-session-worker.c that you will likely need to run
   this program and create the $HOME/.drmc file  after pam_setcred.  Perhaps at 
   the end of the gdm_session_worker_accredit_user() function if it is 
   successful.  There will also probably be some work to load the values in
   to make sure they are set when the user logs in.

2) On my system I have no keyboard layouts defined, so the dialog shows up 
   with no choices.  The code should be smarter to avoid showing dialogs if
   there is nothing to select.

3) I find the "OK" and "Cancel" buttons confusing.  It might make more sense
   to only provide an "OK" button and force the user to pick a choice.  Or
   perhaps instead of "Cancel" it should say something like "Skip setup".

4) It does not seem that the system default language shows up by default in
   the language dialog.  Wouldn't it be better to if each dialog pre-selected
   the actual default setting as the initial default?

5) I notice that the language dialog takes a few seconds to first show-up.  I
   suspect this is because it takes time to figure out what languages are
   installed.  It might be nicer to preload the choices for all the dialogs 
   when the process starts if it isn't too much work.  This would make it 
   faster.

Overall this looks really good, though.

Created attachment 161090 [details]
Attached patch which integrates with gdm and addresses the issues in Brians' mail. 

A couple of observations

- Some language preloading when the gdm_*_chooser_dialog_new() functions now preloads the available session/layouts/languages  and by the time widget is realised, the list entries will be loaded, this fastens the loading

- On rare occasions I have noticed a popup like 'gdm-simple-greeter.desktop' is waiting...' after the gdm-first-time-login-helper runs which then waits for 20secs or so before logging into desktop....

Modifying the following function call by substituting -1 param with 100 (Effectively changing the 25secs lock delay to 100ms) seems to solve it, though I'm not sure why this locks up in some cases...

        reply = dbus_connection_send_with_reply_and_block (connection,
                                                           message,
                                                           -1,
                                                           &error);

in the function

static gboolean
send_dbus_string_method (DBusConnection *connection,
                         const char     *method,
                         const char     *payload)
{


in file gdm-greeter-client.c

Created attachment 161585 [details] [review]
updated patch


This code looks really good to me.  However, there are some issues that I fixed:

1) When I first tried this code, the gdm-first-time-login-helper would not run.
   The problem turned out to be that some of the arguments you pass to this
   program can be NULL.  Surrounding the arguments with double-quotes fixes this
   problem.

2) After fixing the above problem, I noticed that gdm-first-time-login-helper
   would crash with an assert caused when it would call 
   gdm_language_chooser_dialog_set_current* with an empty value.  Fixing the
   code to avoid calling these functions if "" is passed in as an argument
   fixed this.

3) On my system I do not have libxklavier, so gdm-first-time-login-helper
   would create the layout and then destroy it.  The code would then crash
   in queue_column_visibility_update because the "widget" variable was 
   pointing to invalid memory.  Fixing the code to remove the
   idle handler in the gdm_chooser_widget_dispose() function fixed this.

Also note that you built this patch based on the GDM code built for OpenSolaris 
including all the patches that we apply.  Since we build GDM with the patch for 
bug #600914 (gdm-14-last.diff), your patch doesn't apply to the upstream 2.31
code.  I would recommend that you build GDM without patch gdm-14-last.diff if you do further work on updating this patch so the patch applies more cleanly upstream.

I am providing a new patch which fixes all the above issues and which applies more neatly to the master 2.31 git.  This way others can more easily test this also.

Thanks Brian for porting it for upstream. I was trying to repro the timing issue I encountered initially, after 10-15 tries with various system loads (nohup processes running in the background), was unsuccessful. Will continue to look for it...

I had a chat with Ray Strode on IRC about this.  Ray mentioned he had discussed this patch with Jon McCann, and that there was some concern that there is no need to provide the keyboard layout dialog in the first-time-login-helper because users who need a keyboard layout may have needed it to enter their password.

However, after thinking about this, I do not think that this should really be a concern.  Some users who need a special keyboard layout may not need a special layout to enter their password (if their password is all numeric for example), and such users may not have properly selected their settings.  The point of the first-time-login-helper is to help inexperienced users and users who may have overlooked setting the right defaults on their first login.

Asking possibly unnecessary questions in the first-time-login-helper might be a bit annoying (perhaps to more experienced users), but the dialog should only pop-up for a user once (unless perhaps they re-install their system or use a different machine).  I think it is better to help new users step through the process of setting up their defaults even if it might be a little annoying to some users.  It shouldn't be that annoying since the first dialog in the first-time-login-helper has a cancel button, and the user can click on that to avoid seeing the dialogs at all and just accept whatever was already selected on the login GUI.

Also note that the original point of this work was to provide a mechanism for first-time users to set their defaults when in autologin mode.  So, in this case, you really do want to ask all the questions since the user would not have entered a password at all, so they wouldn't have needed to set their layout when entering the password.

Perhaps it would be an improvement if the first-time-login-helper did not request information if the user already picked a non-default option in the login GUI.  Perhaps this would address Ray's and Jon's concerns and make the usability a bit better.

Created attachment 163411 [details] [review]
Patch with addes test for calling gdm-first-time-login-helper only when gdm-session settings is not loaded

This patch https://bugzilla.gnome.org/attachment.cgi?id=163411 was supposed to obsolete https://bugzilla.gnome.org/attachment.cgi?id=161585, some how it did not show it in the list to obsolete...

Based on my discussion with Ray, I think I highlighted all outstanding issues in comment #33.  Is there any more discussion that needs to happen before this patch can go upstream?  It seems in good shape to me.

Created attachment 166556 [details] [review]
Latest patch

Patch with the following fixes in gdm auto login mode

1. Puts the initial window of gdm-first-time-login-helper in the screen center.
2. Replaces the watch cursor with a normal left arrow cursor

Created attachment 169419 [details] [review]
patch for making accessibility work

*** Bug 631162 has been marked as a duplicate of this bug. ***

Created attachment 171744 [details] [review]
newest patch fixing the random gnome-session popup

One of the remaining issues with the patch., a random popup like said in Comment #30

- - On rare occasions I have noticed a popup like 'gdm-simple-greeter.desktop' is waiting...' after the gdm-first-time-login-helper runs which then waits for
20secs or so before logging into desktop....

The newest patch fixes this issue, pl. note that this required to access the /proc directory of the system, the scan_proc function defined in gdm-common.c is tested only on Solaris and in Linux machines some #ifdef'ed code is needed for this to work.

Following is the issue/approach to fix this.

gdm-simple-slave stack trace when the popup show that gdm-simple-slave is waiting on waitpid() from gnome-session when the issue happens. Gnome-session in turn has children including gdm-simple-greeter which has just fired some dbus calls to be serviced by the dbus server in gdm-simple-slave and waiting for reply in a blocked state, gdm-simple-slave can't service the request as it's on waitpid for gnome-session to exit, this becomes a circular wait loop which needs to be timed out.

The solution is to introduce a delay between the kill signal to gnome-session and the wait. After issueing kill signal to gnome-session, we check if we can get the pid of gnome-simple-greeter which is the child of gnome-session, if thats true, we will start waiting for gnome-session exit only when gnome-simple-greeter exits, which can be done by g_child_watch_add function.

I wanted to update this bug report with some issues we are seeing with this code.  We have been delivering GDM built with this patch into our Solaris development builds for some time, and it mostly works really well.  New users 
seem to like having GDM prompt them to set things up and it is also useful for setting up the keyboard layout on install.

However, this patch doesn't seem to be working well.  Right now we are using the patch in comment #37.  However, this patch has the issue mentioned in comment #40.  On occasion, after navigating the first-time-helper dialogs, a pop-up is shown by gnome-session that says: 

  One of the applications is still waiting"
  "Continue" "Cancel" "Log Out"

If the user hits the "Cancel" button, people report that GDM hangs - the user is not asked to enter username or password again.  If the "gdm" user has permission to reboot, then the user can restart the system, but the user can do nothing else.  Since reboot is not enabled for the "gdm" user by default on Solaris, this feels like a hang to most Solaris users.

We tried fixing this with the patch in #40 which did make the dialog no longer appear, but introduced other serious problems.  Using this version of the patch, the GDM slave daemon always left behind a "defunct" process running as the "gdm" user when the user session started and some "gdm" processes would not exit properly.  For example, the user session would start with some processes like metacity or gnome-settings-daemon still running as the "gdm" user, and causing the user to end up with odd configuration issues.  So I do not recommend using the latest patch in comment #40.

I think the issue here is that the code GDM currently uses to set 
session/language/keyboard layout is very convoluted.  When the slave wants to update one of these values, it seems to notify the greeter GUI which updates the GUI elements (so the combo boxes in the panel show the right value).  The slave also seems to actually run the code to manage changing the keyboard layout.  Then the slave signals the slave daemon again which causes the values to get set properly so they will be used.

The issue seems to be that there is a race condition.  If the slave wants to kill the session before this process has completed, you end up with odd behaviors (like the dialog showing up or the keyboard layout not actually set to the chosen value).  This race condition doesn't seem to affect GDM currently because it would be very odd for a user to use the combo box in the panel to change a value and then very quickly initiate the authentication.  Normally users probably set the combo box, then enter their password, allowing the process time to avoid the race.  Also, GDM does a lot in between when the password is entered and when it wants to kill the "gdm" gnome-session (setting up the user session, running the PostLogin and PreSession scripts, etc), so it is possible that this patch tends to trigger the race since it is called immediately before the slave wants to kill the session.

So, the code probably needs some additional work.  Assuming I have grasped the extent of the problem, I can think of two ways to fix this:

1. Fix the slave so that it does not ever try to kill gnome-session until the
   process of updating session/language/layout has completed.
2. It seems the only reason that the slave needs to be involved with this is
   because the slave currently is the only code that knows how to set the 
   keyboard layout.  Perhaps it might make more sense to just make the slave
   daemon directly setup the session, language and layout and avoid using a 
   process that involves multiple process interaction completely.  There is
   really no value in updating the GUI immediately before killing it anyway.

   This seems a simpler and better approach, but would likely involve a bit of 
   redesign since the code seems to be written in a way that assumes multiple 
   processes are always involved with setting these values.

Unfortunately, the code for doing this is convoluted enough that it is hard to
grasp how to best fix this or how to implement either solution.  But I suspect it is not a great deal of work for someone who understands this part of the code, D-Bus, etc.

We are contemplating patching metacity to just suppress the dialog for the "gdm" user.  While a bit of a nasty hack, it seems to address the issue in the meantime.  

So, the patch in this bug report probably needs more work before it goes into upstream GDM.  Since Solaris is likely to just hack gnome-session to avoid the problem, I am hoping that someone may be willing to help finish this work.

we do this now with gnome-initial-setup instead of gdm