[a11y-branch] seeking review for bug 44592: a11y init -- was :Re: [evolution-patches] patches for bug 44592:accessibility initializing codes....
- From: Gilbert Fang <gilbert fang sun com>
- To: Ettore Perazzoli <ettore ximian com>
- Cc: "evolution-patches ximian com" <evolution-patches ximian com>
- Subject: [a11y-branch] seeking review for bug 44592: a11y init -- was :Re: [evolution-patches] patches for bug 44592:accessibility initializing codes....
- Date: 19 Jun 2003 12:35:29 +0800
Hi, Ettore
Thanks for your review.
I have rewritten the patch. Let me show you the changes.
> Also I think I would make it non-conditional to simplify things a bit,
> since it's going into its own branch anyways -- but do as you like. :-)
>
I agree very much that makes a11y non-conditional. In addtion, currently
gtkhtml and gal both build a11y part, so there is no really extra
dependency added for evolution. Thus it also make my patch simpler. :-)
> In the same file, does it matter that you are not shutting down the a11y
> modules on exit?
>
Let me explain it. Idealy, gnome a11y has a key in gconf. Once the key is set,
even when apps is running, a11y should be enabled, vice versa when unset.
That is why a11y part are all modules (*.so) and have special entry with *pre-defined name*
extern void gnome_accessibility_module_init (void);
extern void gnome_accessibility_module_shutdown (void);
These two entries may be invoke in the future by the gnome a11y part or
some gconf stuff.
But in fact, it seems gnome can not deal such stuff idealy. That is why
I have to add explicit initialization code in evolution.
And I do not find any valuable codes or any examples in gnome for
shutdown. Anyway, not doing module_shutdown does not bring any extra
trouble to apps now .
> I re-iterate my opinion that it shouldn't go on HEAD yet. :-)
>
Yes, I see.
> Anyway, here are my comments on the patch:
>
> [init.c]
> +extern void gnome_accessibility_module_init (void);
> +extern void gnome_accessibility_module_shutdown (void);
>
> The declarations are redundant. Also, I don't think Evolution's a11y
> module should pollute the gnome_* namespace; you should just pick one
> prefix and stick with it.
Please see above explanation.
> Actually you should probably use e_a11y_
> instead of evolution_a11y_ since we use the e_ prefix everywhere else.
>
okay, I changed the codes.
> [init.c]
> + fprintf (stderr, "Evolution Accessibility Support
> Extension Module initialized\n");
>
> [init.c]
> + fprintf (stderr, "Evolution Accessibilty Support
> Extension Module shutdown\n");
>
> What's the point of these?
>
It just shows the message that can help developer to confirm the module
is loaded. Gail, libgail-gnome and atk-bridge all have such output.
And I prefer it if it does not matter much trouble to you. :-)
> [configure.in]
> dnl **************************************************
> +dnl * Accessibility support
> +dnl **************************************************
> +AC_ARG_ENABLE(accessibility, [
> --enable-accessibility=[no/yes] Enable support for
> accessibility.],,enable_accessibility=no)
> +if test "x$enable_accessibility" = "xyes"; then
> + AC_DEFINE(ENABLE_ACCESSIBILITY,1,[Enable Accessibility
> support])
> + msg_accessibility=yes
> + PKG_CHECK_MODULES(ACCESSIBILITY, atk)
> + AC_SUBST(ACCESSIBILITY_CFLAGS)
> + AC_SUBST(ACCESSIBILITY_LIBS)
> +else
> + msg_accessibility=no
> +fi
> +AM_CONDITIONAL(ENABLE_ACCESSIBILITY, test
> "x$enable_accessibility" = "xyes")
>
> Just an aesthetical nitpick, but I think it would look nicer if we
> abbreviated ACCESSIBILITY with A11Y everywhere.
>
> --- Makefile.am 26 Mar 2003 23:02:26 -0000 1.80
> +++ Makefile.am 12 Jun 2003 06:55:01 -0000
> @@ -22,6 +22,10 @@
> intltool-extract.in \
> $(pkgconfig_DATA:.pc=.pc.in)
>
> +if ENABLE_ACCESSIBILITY
> +ACCESSIBILITY_SUBDIR = a11y
> +endif
> +
> SUBDIRS = \
> data \
> e-util \
> @@ -43,6 +47,7 @@
> views \
> wombat \
> tools \
> + $(ACCESSIBILITY_SUBDIR) \
> help \
> po
>
> I think this is going to confuse Automake when making dist. You
> probably want to put the conditional in the subdir only, so the subdir
> is always built but it doesn't actually create a library when a11y is
> disabled.
>
Please see above. Now it is non-conditional, so no problem here.
> [main.c]
> + sub = g_strconcat ("gtk-2.0/modules", G_DIR_SEPARATOR_S,
> fname, NULL);
>
> Here (and in the other cases) you can use g_build_filename() instead.
>
okay, thank you.
> [main.c]
> +static gboolean
> +accessibility_invoke_module (GnomeProgram *program,
> + const char *libname,
> + gboolean init)
>
> Another nitpick: in the shell I like to not namespace local functions.
> So I would name this invoke_a11y_module() instead...
>
> [main.c]
> + if ((env_var = g_getenv (GNOME_ACCESSIBILITY_ENV)))
>
> Please split this into two statements.
>
> [main.c]
> + do_init = atoi (env_var);
> + else
> + do_init = gconf_client_get_bool (
> + gconf_client_get_default (),
> + GNOME_ACCESSIBILITY_KEY, NULL);
>
> This leaks a GConfClient ref. You need to g_object_unref() the return
> value from gconf_client_get_default() when you are done.
>
yes, thank you for guiding.
> I hope this helps,
It really helps me much. Thank you again.
If the patch is okay, can I check it into a11y-branch?
Best Regards
Gilbert
Index: configure.in
===================================================================
RCS file: /cvs/gnome/evolution/configure.in,v
retrieving revision 1.589
diff -u -r1.589 configure.in
--- configure.in 2 Jun 2003 20:19:18 -0000 1.589
+++ configure.in 19 Jun 2003 03:01:20 -0000
@@ -251,6 +251,13 @@
dnl **************************************************
+dnl * Accessibility support
+dnl **************************************************
+PKG_CHECK_MODULES(A11Y, atk)
+AC_SUBST(A11Y_CFLAGS)
+AC_SUBST(A11Y_LIBS)
+
+dnl **************************************************
dnl * IPv6 support
dnl **************************************************
AC_ARG_ENABLE(ipv6, [ --enable-ipv6=[no/yes] Enable support for resolving IPv6 addresses.],,enable_ipv6=no)
@@ -1285,6 +1292,7 @@
AC_OUTPUT([ po/Makefile.in
Makefile
+a11y/Makefile
addressbook/Makefile
addressbook/gui/Makefile
addressbook/gui/component/Makefile
Index: Makefile.am
===================================================================
RCS file: /cvs/gnome/evolution/Makefile.am,v
retrieving revision 1.81
diff -u -r1.81 Makefile.am
--- Makefile.am 12 Jun 2003 21:22:41 -0000 1.81
+++ Makefile.am 19 Jun 2003 03:01:20 -0000
@@ -44,6 +44,7 @@
views \
wombat \
tools \
+ a11y \
help \
po
Index: shell/main.c
===================================================================
RCS file: /cvs/gnome/evolution/shell/main.c,v
retrieving revision 1.123
diff -u -r1.123 main.c
--- shell/main.c 19 May 2003 19:41:05 -0000 1.123
+++ shell/main.c 19 Jun 2003 03:01:20 -0000
@@ -35,6 +35,7 @@
#include <gconf/gconf-client.h>
+#include <gmodule.h>
#include <gtk/gtkalignment.h>
#include <gtk/gtkframe.h>
#include <gtk/gtklabel.h>
@@ -91,6 +92,9 @@
extern char *evolution_debug_log;
+#define GNOME_ACCESSIBILITY_ENV "GNOME_ACCESSIBILITY"
+#define GNOME_ACCESSIBILITY_KEY "/desktop/gnome/interface/accessibility"
+
static GtkWidget *
quit_box_new (void)
@@ -534,6 +538,111 @@
g_static_mutex_lock (&segv_mutex);
}
+static char *
+find_a11y_module (GnomeProgram *program, const char *libname)
+{
+ char *sub;
+ char *path;
+ char *fname;
+ char *retval;
+
+ fname = g_strconcat (libname, "." G_MODULE_SUFFIX, NULL);
+ sub = g_build_filename ("gtk-2.0","modules", fname, NULL);
+
+ path = gnome_program_locate_file (
+ program, GNOME_FILE_DOMAIN_LIBDIR, sub, TRUE, NULL);
+
+ g_free (sub);
+
+ if (path)
+ retval = path;
+ else {
+ sub = g_build_filename ("evolution", BASE_VERSION, fname, NULL);
+ path = gnome_program_locate_file (
+ program, GNOME_FILE_DOMAIN_LIBDIR, sub, TRUE, NULL);
+ g_free (sub);
+
+ if (path)
+ retval = path;
+ else
+ retval = gnome_program_locate_file (
+ program, GNOME_FILE_DOMAIN_LIBDIR, fname, TRUE, NULL);
+ }
+ g_free (fname);
+
+ return retval;
+}
+
+
+static gboolean
+invoke_a11y_module (GnomeProgram *program,
+ const char *libname,
+ gboolean init)
+{
+ GModule *handle;
+ void (*invoke_fn) (void);
+ const char *method;
+ gboolean retval = FALSE;
+ char *module_name;
+
+ if (init)
+ method = "gnome_accessibility_module_init";
+ else
+ method = "gnome_accessibility_module_shutdown";
+
+ module_name = find_a11y_module (program, libname);
+
+ if (!module_name) {
+ g_warning ("Accessibility: failed to find module '%s' which "
+ "is needed to make this application accessible",
+ libname);
+
+ } else if (!(handle = g_module_open (module_name, G_MODULE_BIND_LAZY))) {
+ g_warning ("Accessibility: failed to load module '%s': '%s'",
+ libname, g_module_error ());
+
+ } else if (!g_module_symbol (handle, method, (gpointer *)&invoke_fn)) {
+ g_warning ("Accessibility: error library '%s' does not include "
+ "method '%s' required for accessibility support",
+ libname, method);
+ g_module_close (handle);
+
+ } else {
+ retval = TRUE;
+ invoke_fn ();
+ }
+
+ g_free (module_name);
+
+ return retval;
+}
+
+static void
+init_a11y (GnomeProgram *program)
+{
+ gboolean do_init;
+ const char *env_var;
+ GConfClient *gconf;
+
+ do_init = FALSE;
+
+ env_var = g_getenv (GNOME_ACCESSIBILITY_ENV);
+ if (env_var) {
+ do_init = atoi (env_var);
+ }
+ else {
+ gconf = gconf_client_get_default (),
+ do_init = gconf_client_get_bool ( gconf, GNOME_ACCESSIBILITY_KEY, NULL);
+ g_object_unref (G_OBJECT(gconf));
+ }
+
+ if (do_init) {
+ invoke_a11y_module (program, "libgal-a11y-2.0", TRUE);
+ invoke_a11y_module (program, "libevolution-a11y", TRUE);
+ }
+}
+
+
int
main (int argc, char **argv)
{
@@ -573,6 +682,8 @@
GNOME_PARAM_POPT_TABLE, options,
GNOME_PARAM_HUMAN_READABLE_NAME, _("Evolution"),
NULL);
+
+ init_a11y (program);
if (start_online && start_offline) {
fprintf (stderr, _("%s: --online and --offline cannot be used together.\n Use %s --help for more information.\n"),
--- /dev/null 2002-08-31 07:31:37.000000000 +0800
+++ a11y/init.c 2003-06-18 17:33:51.000000000 +0800
@@ -0,0 +1,75 @@
+/* -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 8; tab-width: 8 -*- */
+/* init.c
+ *
+ * Copyright (C) 2003 Ximian, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ * Author: Gilbert Fang <gilbert fang sun com> Sun Microsystem Inc., 2003
+ *
+ */
+
+#include <config.h>
+
+#include <atk/atkregistry.h>
+
+#include <stdio.h>
+
+/* Static functions */
+
+static gboolean initialized = FALSE;
+
+extern void gnome_accessibility_module_init (void);
+extern void gnome_accessibility_module_shutdown (void);
+
+void
+e_a11y_init (void)
+{
+ if (initialized)
+ return;
+
+ /* atk registry stuff here, need implementation*/
+
+ initialized = TRUE;
+
+ fprintf (stderr, "Evolution Accessibility Support Extension Module initialized\n");
+}
+
+void
+gnome_accessibility_module_init (void)
+{
+ e_a11y_init();
+}
+
+void
+gnome_accessibility_module_shutdown (void)
+{
+ if (!initialized)
+ return;
+
+ /* atk un-registry stuff here need implementation*/
+
+ initialized = FALSE;
+ fprintf (stderr, "Evolution Accessibilty Support Extension Module shutdown\n");
+
+}
+
+int
+gtk_module_init (gint *argc, char** argv[])
+{
+ e_a11y_init ();
+
+ return 0;
+}
--- /dev/null 2002-08-31 07:31:37.000000000 +0800
+++ a11y/Makefile.am 2003-06-18 17:31:42.000000000 +0800
@@ -0,0 +1,19 @@
+
+INCLUDES = \
+ -DPREFIX=\"$(prefix)\" \
+ -DSYSCONFDIR=\"$(sysconfdir)\" \
+ -DDATADIR=\"$(datadir)\" \
+ -DLIBDIR=\"$(libdir)\" \
+ -DG_LOG_DOMAIN=\"evolution-a11y\" \
+ -I$(top_srcdir) \
+ -DG_DISABLE_DEPRECATED \
+ -DLIBGNOME_DISABLE_DEPRECATED \
+ $(A11Y_CFLAGS)
+
+privlib_LTLIBRARIES = libevolution-a11y.la
+
+libevolution_a11y_la_SOURCES = \
+ init.c
+
+libevolution_a11y_la_LIBADD = \
+ $(A11Y_LIBS)
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/ChangeLog,v
retrieving revision 1.1211
diff -u -r1.1211 ChangeLog
--- ChangeLog 17 Jun 2003 03:40:09 -0000 1.1211
+++ ChangeLog 19 Jun 2003 03:20:36 -0000
@@ -1,3 +1,8 @@
+2003-06-18 Gilbert Fang <gilbert fang sun com>
+
+ * configure.in: Added a11y support.
+ * Makefile.am: Added a11y subdir. (bug #44592)
+
2003-06-17 Not Zed <NotZed Ximian com>
* NEWS: Updated for current mail stuff.
--- /dev/null 2002-08-31 07:31:37.000000000 +0800
+++ a11y/ChangeLog 2003-06-19 11:49:05.000000000 +0800
@@ -0,0 +1,4 @@
+2003-06-18 Gilbert Fang <gilbert fang sun com>
+
+ * init.c: Initial new file for libevolution-a11y.so framework.
+ * Makefile.am: Initial new file (bug #44592)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
