Created attachment 45807 [details] [review]
Adds Random Graphical Themes Option

Added to CVS head.  By the way, would you be interested in adding a tab for
configuring the Xserver into gdmsetup.c?  If so, let me know.  This would be a
bit of work, but would make gdmsetup much more useful.

I'd be happy to give it a shot.

Created attachment 46186 [details]
Simple XServer Configuration

Were you thinking something like this?

Created attachment 46187 [details]
Complex XServer Configuration

Or something like this?

Hmm.  Adding a tab to configure the Xserver is something that would be very
useful for gdmsetup to support.  Lots of people ask for it.  

However, what I was originally asking for was something else.  I sent you an
email explainging that it would be nice to be able to modify the configuration
settings for the Xserver in the gdm.conf file.  Perhaps you missed that email? 
If so, I'm attaching the text below.

---

Many users complain that gdmsetup doesn't allow users to modify
the gdm2 Xserver configuration options.  Refer:

   http://bugzilla.gnome.org/show_bug.cgi?id=89421

In other words, the gdm.conf file allows users to specify the following:

0=Standard
[displaynum]=[servertype]
[...]

[server-Standard]
name=Standard server
command=/usr/X11/bin/X -audit 0
flexible=true

[server-Terminal]
name=Terminal server
command=/usr/X11/bin/X -audit 0 -terminate
flexible=false
handled=false

[server-Chooser]
name=Chooser server
command=/usr/X11/bin/X -audit 0
flexible=false
chooser=true

It would be nice to be able to define these blocks via gdmsetup.
From a GUI perspective this will be a bit tricky since we need an
interface where the user can specify the displaynum and servertype
for each display they want GDM to manage.

Also, users can create their own "[server-myserver]" section(s) and specify
that it should be used when starting the Xserver for a specific display.
This might be a bit tricky to support via the GUI.  Though we could
take this in stages.  Perhaps first support just allowing the user
to modify the existing 3 types, and specify which displays should be
managed with which servers.  Then allowing users to define their own
new session types could be a second round.

This would be useful because then users could specify that the command
to start the Xserver, for example, is different on their machine, or
requires special flags (for a11y perhaps).  It would allow the sysadmin
to specify that a particular display requires a11y flags because the
user on that display has a11y needs, or make it easy to setup a display
for using a different Xserver, etc.

It might also be nice to allow the "Xnest" gdm.conf value to be
specified on the same tab (it is in the "[daemon]" section). 

Oh wait, sorry, I only looked at the Simple image when I posted my last comment 
(though it's probably good to capture that information in bugzilla too).  

I was thinking of something more like the Complex configuration GUI that you
wrote.  It looks awesome by the way.  It would also be nice to support being 
able to specify the lines that look like this:

 0=Standard
 1=Standard
 2=Terminal -query serverhostname
 3=Terminal -indirect serverhostname
 4=Chooser

That's the section of the config file where you can specify what type of Xserver
to start for each display number.

Though this could be done in more than one stage.  Implementing what you've
done in the Complex image as stage 1, then adding the ability to define what
xserver to use for each display could be stage 2.  Then adding the ability
to define new server types (ones in addition to the default ones of
server-Standard, server-Terminal, and server-Chooser) could be stage 3.

One more thing.  We should probably talk about the new Xserver config tab in bug
#89421, since that's the bug that relates to this feature.

Also, I reopened this bug because I think there's a bug with the new "random
theme" feature.  Note that the "Delete Theme" button is setup so that it is
insensitive when the checked theme is selected.  This is to ensure that the
program doesn't enter the invalid state where no theme is checked.

In random theme mode, this should work differently.  In this case, it is okay 
to delete a checked theme, but the "Delete Theme" should become insensitive 
if there is only one checked theme and the user selects it.  There are 
probably also some issues that need to be worked out if the user does 
something like:

1. Has one theme selected
2. Switches to random theme mode
3. Deletes the theme that was selected in "one theme" mode.
4. Goes back to one theme mode.
5. gdmsetup should not allow the user to keep the "one theme" mode choice
   unless a theme is checked.  Perhaps we just need to ensure that the 
   user can't delete all the themes, and the first theme could be checked
   by default if the user deleted the theme that was previously checked.

Could you look into this?

You are right the sensitivity definately doesn't work quite right.  I'll look
into that.  I've started working on coding the XServer addition as well.  I'll
post more about that under the #89421 bug.

Created attachment 47582 [details] [review]
Patch to fix delete button sensitivity under themes tab.

In this patch I set the sensitivity of the delete button to work as follows:
Sensitivity is dependent strictly on which mode (Random or Single Theme Mode)
is selected.  In other words, if Random mode is selected the sensitivity of the
delete button is not affected by which theme is selected in Single Theme mode,
and vice versa.
The delete button is not sensitive if the current theme selected is acitve. 
For a theme to be deleted, it must be inactive, in which case the delete button
will become sensitive as soon as it is deactivated.

Looking at your patch, it doesn't really seem to address the 2 underlying
problems.  Let me know if I'm missing something.

1) There should *always* be at least one theme selected.  In random mode,
   the code should probably ensure that at least one theme is always checked
   and should not allow the user to delete the theme if it is the only theme 
   that is checked.
2) What happens if I am in "One Theme" mode and have theme "xyz" checked.  Then 
   I go to Random mode and delete theme "xyz" and then go back to "One Theme"
   mode.  I now won't have a selected theme.

Perhaps it would be better to make the button always insensitive if the theme
is checked in both "One Theme" and "Random Theme" mode.  This way the button
works the same way in both modes.  It seems reasonable that we expect the user
to first uncheck the theme before deleting it.  This way we don't need special
code for "One Theme" versus "Random Theme" mode.  This way we don't have to
worry about the user deleting all their themes.

We still have the problem #2 above.  We probably just need some code that
looks to see if no theme is checked and just automatically check the first
theme.  If the user doesn't like this choice, they can change it.  

The following problems...

1) Yes, a user could delete all their themes, but a user can do that fine
without gdmsetup (from commandline) and still tell gdmsetup that they want the
themed greeter.  Regardless, the greeter realizes it can't display a theme
because none exist and then it warns the user and starts with GTK+ Greeter
instead.  This doesn't create a problem, the program shouldn't crash, it just
lets the user know, you haven't properly selected your settings.  This makes
sense because, if the user hasn't selected a theme, then they don't get a theme.
 However if a user hasn't deleted all their themes, but hasn't selected one
either, then gdm defaults to Circles so this isn't a problem either.

2) If a user removes the theme "Circles" from gdm's theme directory and loads
the gdmsetup program they are greated with the same problem.  Gdm handles it
gracefully by switching to GTK+ Greeter and informing the user of the problem.

Problems with other implementations:
If a users has "Random Theme" mode selected, they should always have to have a
theme selected lets say I only had say the "Circles" theme selected, but I
wanted "foobar" selected instead.  I shouldn't have to select "foobar", then
deselect "Circles" afterwards.  I should be able to deselect "Circles" then
select "foobar".

Now lets say "Circles" is selected under "One Theme" and "Foo" is selected under
"Random Theme".  If I'm in "Random Theme" mode, then why shouldn't I be able to
delete "Circles", it wouldn't make any sense to the user for it not to let me
delete it.  They wouldn't know why they can't delete "Circles" but coulde delete
"Bar" for example, since it looks to them like neither is selected.

To me, my way seems the least confusing to the user, and to be honest gives them
the most control, without allowing them to jepordize the integrity gdm.  Perhaps
what we should have is that if no themes exist, that the option to select
"Themed Greeter" is insensitive and is automatically set back to GTK+ Greeter?

Thanks for testing that GDM behaves well even if the "Themed Greeter" tab
doesn't have a setting for the theme.  I sort of disagree with the problems you
describe above for the following reasons:

1) "Random Theme" mode is *only* useful if the user has more than one theme
   selected.  Therefore, the user should rarely have a situation where they
   only have one theme selected in Random Theme mode.

2) I think it's okay to delete "Circles" if it is selected under "One Theme"
   mode and the user is in "Random Theme" mode.  I was suggesting that if
   the user switches back to "One Theme" mode, that gdm2 just go ahead and
   select the first theme so at least one is selected.

Perhaps we could make the UI less confusing.  Why do we need two sets of
checkboxes anyway?  Might it make more sense to just to have one set of
checkboxes?  When the user selects "Random Theme" mode, it lets the user
check more than one.  When the user selects "One Theme" mode, it lets the
user check only one.  If the user is in "One Theme" mode and switches to
"Random Theme" mode, all of them can be checked.  If the user unchecks all
of them but one, it can automatically move them back to "One Theme" mode.
This might work a bit more cleanly and avoid the "Delete Theme" problem.

What do you think?

Oh, your idea of making the "Themed Greeter" tab insensitive if the user has no
installed themes is not a good idea, since the user can install new themes on
the "Themed Greeter" tab.  However, it would be a good idea to make the user
unable to change the greeter to "Themed Greeter" on the "General" tab unless at
least one theme is installed.

Yes this is what I was referring to in my last post about the insensitive (not
the tab).

However I still don't like the functionality you described.  I don't think a
program should change your settings for you and should allow the user the most
versatility without compromising integrity.  For instance, what if I had 3
themes selected as such...

X  Circles
X  Foo
X  Bar
_  Cow
_  Penguin

but I wanted

X  Circles
_  Foo
_  Bar
X  Cow
X  Penguin

If I unselect Foo and Bar, then the program swaps to One Theme mode.  After
which, I have to swap it back to Random Theme mode and reselect the themes I
want.  In my opinion, the UI should not require you do things in a particular
order to obtain a desired result.  This is the same argument I have about having
to select a different theme before deselecting another theme to avoid the user
"not selecting" a theme (in Random Theme mode).  The only decent option I would
see to force at least one theme is to stop the user from deleting the last theme
and default to it, however I think a user should be able to delete ALL themes if
they don't want to use the Themed Greeter.  So I think the best option would be
just to allow the functionality that I've already implemented, but if a user has
deleted all the themes or has not selected a theme to automatically switch back
to GTK+ Greeter and if they have deleted all the themes then to make the
Combobox for Themed Greeter insensitive.

darkfrog@remotehost awaing_your_response #

Okay, you've convinced me.  I agree your original patch to fix "Delete Theme"
button sensitivity is fine, so please go ahead and commit it.  The other fixes
you suggest would be good to do also.  I'll leave this bug open until these 
two issues are addressed:

+ if a user has deleted all themes or has not selected a theme to automatically
  switch to Gtk+ Greeter
+ if the user has deleted all themes, the combobox for themed greeter becomes
  insensitive

Thanks Brian, but I don't have cvs privileges to commit.

Okay, patch in CVS head.

Should this report be closed then? Or maybe the patch marked as commited at least?

Yes, marked as commited.  Thanks.

Marking this bug as fixed.  I think all the issues raised in this bug were addressed, aside from the general selection issues of the themes discussed in bug 
320802/