Attaching a patch which exports GTK_MODULES to gail:atk-bridge if the
accessibility key is turned on, while starting gnome-session.

Created attachment 32816 [details] [review]
Proposed patch

Marking AP1 to reflect accessibility impact.

Apologies for spam-- ensuring Sun a11y team are cc'ed on all current a11y bugs.
 Filter on "SUN A11Y SPAM" to ignore.

Created attachment 33238 [details] [review]
Modified the patch so as not to override the env variable

Mark: ok to commit?

Do we actually need this?  See bug #117236 for a more dynamic way of doing
things.  GTK+ supports an XSETTING via which you can tell running apps to load
accessibility modules.

We do need this, as far as I can tell.   But I still don't see how the approach
in 117236 works, i.e. I don't think it's actually 

... don't think it's compatible with our current solution which is to set
GTK_MODULES.  We need to retain compatibility with that previous solution.

Federico, you marked the patch 'needs work' but failed to explain your specific
issues with the patch... ;-)

Created attachment 38850 [details] [review]
Patch fixes the bug

modifiying the previous patch. moved the setting of the env variable before
gnome_program_init() so that b-a-s also gets it. solves issues raised in bug
#166534

leaving the patch in needs-work state based on Fedrico's previous comment, but
would like it to expanded to specifics in case we should be fixing in that approach.

please remove "needs work" as the current patch looks correct to me, it's what
we are using for Sun builds, and as I said in comments #8 through #10, the
XSETTING proposal doesn't maintain compatibility with our existing solution, nor
is it explained how it would work.

Comment on attachment 38850 [details] [review]
Patch fixes the bug

Can this patch please be applied to CVS?  It solves a long-standing
accessibility issue with applications which do not use gnome_program_init.

As a user of access technologies as well as someone working on access 
technology for the gnome desktop I'm finding this issue to be quite 
frustrating.  It would be great if this patch could be applied as it would mean 
one less thing a blind person has to fight with when trying to get up and 
running with a new install.  

OK, I'm confused.

- If I enable a11y in GNOME, restart my session, and run a gtk-only app that
doesn't use gnome_program_init(), will my app be a11y-enabled?

- Does Muktha's patch fix that?

- Or is your patch only for GTK+ stuff that gets launched through
bonobo-activation-server?

- Can this be done without setting an env. var. in gnome-session, but solely
with the XSETTING that was added?

- If so, is gnome-settings-daemon not turning on that setting or something?

> - If I enable a11y in GNOME, restart my session, and run a gtk-only app that
>   doesn't use gnome_program_init(), will my app be a11y-enabled?

I don't think so

> - Does Muktha's patch fix that?

I think so

> - Or is your patch only for GTK+ stuff that gets launched through
>   bonobo-activation-server?

No, its for all apps not using gnome_program_init()

> - Can this be done without setting an env. var. in gnome-session, but solely
>   with the XSETTING that was added?

The "Gtk/Modules" XSETTING ?

I think we could do it this way, but my worry is that these modules would have
to be loaded at the very start of each program because we can't handle
gtk_widget_get_accessible() returning NULL at one point in the program and
returning an AtkObject later.

I'm not even sure what would happen if you loaded the a11y modules into a
running app ...

> - If so, is gnome-settings-daemon not turning on that setting or something?

So, we could do it in g-s-d, but we'd need to be confident that a) the XSETTING
would be set before any apps are launched and b) that apps don't get hosed if we
change the XSETTING while they are running.

If we did it this way, we could probably get rid of the a11y stuff in
gnome_program_init() too ...

Also, right now g-s-d only runs per-user, per-machine rather than per-display
(bug #94049) so a11y wouldn't work on multi-display systems. Now would be a good
time to fix that.



As far as patch review goes:

  - Why are you doing this before gnome_program_init() ? gnome-session doesn't
    need to load the a11y modules this way because its done in 
    gnome_program_init(), right?

  - Why aren't we loading the "gail-gnome" module? Is the idea that if the
    app doesn't use gnome_program_init(), it musn't be using Bonobo controls?

  - Move the code for setting GTK_MODULES into gsm-at.c into a function like:

void
gsm_at_set_gtk_modules (void)
{

  GSList      *modules_list;
  GSList      *l;
  const char  *old;
  char       **modules;
  char        *new;
  gboolean     found_gail;
  gboolean     found_atk_bridge;
  int          n;

  n                = 0;
  modules_list     = NULL;
  found_gail       = FALSE;
  found_atk_bridge = FALSE;

  if ((old = g_getenv ("GTK_MODULES")) != NULL)
    {
      modules = g_strsplit (old, ":", -1);
      for (n = 0; modules[n]; n++)
        {
          if (!strcmp (modules[i], "gail"))
            found_gail = TRUE;
          else if (!strcmp (modules[i], "atk_bridge"))
            found_atk_bridge = TRUE;

          modules_list = g_slist_prepend (modules_list, modules[n]);
          modules[n] = NULL;
        }

      g_free (modules);
    }

  if (!found_gail)
    {
      modules_list = g_slist_prepend (modules_list, "gail");
      ++n;
    }

  if (!found_atk_bridge)
    {
      modules_list = g_slist_prepend (modules_list, "atk-bridge");
      ++n;
    }

  modules = g_new (char *, n + 1);
  modules[n--] = NULL;
  for (l = modules_list; l; l = l->next)
    modules[n--] = l->data;

  g_setenv ("GTK_MODULES", 
            g_strjoinv (":", modules),
            TRUE);

  g_strfreev (modules);
  g_slist_free (modules_list);
}

  - and then put

  if (a_t_support)
    gsm_at_set_gtk_modules ();

    just before gsm_gsd_start ();

Thanks Mark:

>> - If I enable a11y in GNOME, restart my session, and run a gtk-only app that
>>   doesn't use gnome_program_init(), will my app be a11y-enabled?

>I don't think so

Correct.

>> - Does Muktha's patch fix that?

>I think so

Yes.
...

>I'm not even sure what would happen if you loaded the a11y modules into a
>running app ...

Probably not what you would want.  ATM a11y isn't dynamically loadable, sorry,
that's a good RFE.

Sounds like you (Mark) have a very good handle on the issues.
...
>
>  - Why aren't we loading the "gail-gnome" module? Is the idea that if the
>    app doesn't use gnome_program_init(), it musn't be using Bonobo controls?

Yes.  AFAIK this is currently true of all Bonobofied apps.  

Hi Mark,

1. The file gsm-at.c is not present in the gnome-session sources. Hence I have 
   put the code which sets the GTK_MODULES into gsm-at-startup.c. I guess you 
   meant the same file.

2. By the time gnome_program_init() is over in gnome-session the b-a-s is  
   started and does not have the GTK_MODULES. Hence any gtk apps that get 
   invoked by the applets are not accessible (#166534). So I have moved the env 
   setting before gnome_program_init() in gnome-session. 
   Please let know if I am missing something.

I'm attaching the modified patch.

Created attachment 55130 [details] [review]
Modified patch

Please go ahead and commit the patch in comment #20.

I commited this.