Merge lp:~apinheiro/unity/a11y-improve-window-event-emission into lp:unity

[Alejandro Piñeiro]

[Alejandro Piñeiro (apinheiro)]

BTW, this branch is already being tested by the users, as it is included on this branch:

https://launchpad.net/~apinheiro/+archive/unity-extra-a11y

I already received some positive feedback from ubuntu-accessibility mailing list

[Alex Launi (alexlauni)]

API, could you please also include some tests for these merges? We're focusing on increaing our test code coverage and the a11y layer is an important part to be tested

review: Needs Fixing

[Alejandro Piñeiro (apinheiro)]

> API, could you please also include some tests for these merges? We're focusing
> on increaing our test code coverage and the a11y layer is an important part to
> be tested

Yes it is true that a lot of new features were included without no new tests. Anyway, as most of those branches are already really big, and in order to keep things clean, could it possible to add tests on a different merge proposal?. In that way I could start to work on adding tests for a more global a11y support.

For example this branch is basically a improvement of previous branch, so I feel that include also tests there would be like mixing different things.

And after all, it would be good to check if it would be possible to move the a11y tests to the normal a11y directory. When I created those tests, I couldn't add them to the tests directory because there I couldn't included some Unity headers (I don't remember the details, I think that are detailed on a mail or on other merge proposal).

[Alex Launi (alexlauni)]

Ok, sounds fair. Could you please propose some test branches as soon as possible?

review: Approve

[Unity Merger (unity-merger)]

No proposals found for merge of lp:~apinheiro/unity/a11y-quicklist into lp:unity.

=== modified file 'plugins/unityshell/src/nux-base-window-accessible.cpp'
--- plugins/unityshell/src/nux-base-window-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/nux-base-window-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -46,15 +46,6 @@
 #include <Nux/Area.h>
 #include <Nux/Layout.h>
 
-enum
-{
-  ACTIVATE,
-  DEACTIVATE,
-  LAST_SIGNAL
-};
-
-static guint signals [LAST_SIGNAL] = { 0, };
-
 /* GObject */
 static void nux_base_window_accessible_class_init(NuxBaseWindowAccessibleClass* klass);
 static void nux_base_window_accessible_init(NuxBaseWindowAccessible* base_window_accessible);
@@ -65,20 +56,19 @@
 static AtkObject* nux_base_window_accessible_get_parent(AtkObject* obj);
 static AtkStateSet* nux_base_window_accessible_ref_state_set(AtkObject* obj);
 
-/* private */
-static void         on_change_keyboard_receiver_cb(AtkObject* accessible,
-                                                   gboolean focus_in);
-
-G_DEFINE_TYPE(NuxBaseWindowAccessible, nux_base_window_accessible,  NUX_TYPE_VIEW_ACCESSIBLE)
+/* AtkWindow.h */
+static void         atk_window_interface_init(AtkWindowIface* iface);
+
+
+G_DEFINE_TYPE_WITH_CODE(NuxBaseWindowAccessible, nux_base_window_accessible,
+                        NUX_TYPE_VIEW_ACCESSIBLE,
+                        G_IMPLEMENT_INTERFACE(ATK_TYPE_WINDOW,
+                                              atk_window_interface_init))
 
 struct _NuxBaseWindowAccessiblePrivate
 {
   /* Cached values (used to avoid extra notifications) */
   gboolean active;
-
-  gboolean key_focused;
-  gboolean child_key_focused;
-  /* so active = key_focused || child_key_focused */
 };
 
 #define NUX_BASE_WINDOW_ACCESSIBLE_GET_PRIVATE(obj) \
@@ -97,45 +87,6 @@
   atk_class->ref_state_set = nux_base_window_accessible_ref_state_set;
 
   g_type_class_add_private(gobject_class, sizeof(NuxBaseWindowAccessiblePrivate));
-
-  /**
-   * BaseWindow::activate:
-   * @nux_object: the object which received the signal
-   *
-   * The ::activate signal is emitted when the window receives the key
-   * focus from the underlying window system.
-   *
-   * Toolkit implementation note: it is used when anyone adds a global
-   * event listener to "window:activate"
-   */
-  signals [ACTIVATE] =
-    g_signal_new("activate",
-                 G_TYPE_FROM_CLASS(klass),
-                 G_SIGNAL_RUN_LAST,
-                 0, /* default signal handler */
-                 NULL, NULL,
-                 g_cclosure_marshal_VOID__VOID,
-                 G_TYPE_NONE, 0);
-
-  /**
-   * BaseWindow::deactivate:
-   * @nux_object: the object which received the signal
-   *
-   * The ::deactivate signal is emitted when the window loses key
-   * focus from the underlying window system.
-   *
-   * Toolkit implementation note: it is used when anyone adds a global
-   * event listener to "window:deactivate"
-   */
-  signals [DEACTIVATE] =
-    g_signal_new("deactivate",
-                 G_TYPE_FROM_CLASS(klass),
-                 G_SIGNAL_RUN_LAST,
-                 0, /* default signal handler */
-                 NULL, NULL,
-                 g_cclosure_marshal_VOID__VOID,
-                 G_TYPE_NONE, 0);
-
 }
 
 static void
@@ -166,21 +117,9 @@
 nux_base_window_accessible_initialize(AtkObject* accessible,
                                       gpointer data)
 {
-  nux::Object* nux_object = NULL;
-  nux::BaseWindow* bwindow = NULL;
-
   ATK_OBJECT_CLASS(nux_base_window_accessible_parent_class)->initialize(accessible, data);
 
-  accessible->role = ATK_ROLE_WINDOW;
-
-  nux_object = nux_object_accessible_get_object(NUX_OBJECT_ACCESSIBLE(accessible));
-  bwindow = dynamic_cast<nux::BaseWindow*>(nux_object);
-
-  /* This gives us if the window has the underlying key input */
-  bwindow->begin_key_focus.connect(sigc::bind(sigc::ptr_fun(on_change_keyboard_receiver_cb),
-                                              accessible, TRUE));
-  bwindow->end_key_focus.connect(sigc::bind(sigc::ptr_fun(on_change_keyboard_receiver_cb),
-                                            accessible, FALSE));
+  atk_object_set_role(accessible, ATK_ROLE_WINDOW);
 }
 
 static AtkObject*
@@ -219,76 +158,46 @@
   return state_set;
 }
 
-/* private */
+/* AtkWindow */
 static void
-check_active(NuxBaseWindowAccessible* self)
-{
-  gint signal_id;
+atk_window_interface_init(AtkWindowIface* iface)
+{
+  /* AtkWindow just define signals at this moment */
+}
+
+/* public */
+/*
+ * Checks if we are the active window.
+ */
+void
+nux_base_window_accessible_check_active(NuxBaseWindowAccessible* self,
+                                        nux::BaseWindow* active_window)
+{
+  const gchar* signal_name;
   gboolean is_active;
   nux::Object* nux_object = NULL;
+  nux::BaseWindow* bwindow = NULL;
+
+  g_return_if_fail(NUX_IS_BASE_WINDOW_ACCESSIBLE(self));
 
   nux_object = nux_object_accessible_get_object(NUX_OBJECT_ACCESSIBLE(self));
-  if (nux_object == NULL) /* defunct */
+  bwindow = dynamic_cast<nux::BaseWindow*>(nux_object);
+  if (bwindow == NULL) /* defunct */
     return;
 
-  /* FIXME: this method is not infalible, as we lose some
-     window->deactivate, specifically on the Dash, so this could be
-     improved. As we check the active window using UBus messages we
-     could use that information, but would make complex the
-     quicklist.
-
-     Anyway, the relevant message is the activate window, it is ok if
-     we lose some deactivate methods for the moment */
-  is_active = (self->priv->key_focused || self->priv->child_key_focused);
+  is_active = (bwindow == active_window);
 
   if (self->priv->active != is_active)
   {
     self->priv->active = is_active;
 
     if (is_active)
-      signal_id = ACTIVATE;
+      signal_name = "activate";
     else
-      signal_id = DEACTIVATE;
+      signal_name = "deactivate";
 
     atk_object_notify_state_change(ATK_OBJECT(self),
                                    ATK_STATE_ACTIVE, is_active);
-    g_signal_emit(self, signals [signal_id], 0);
-  }
-}
-
-static void
-on_change_keyboard_receiver_cb(AtkObject* object,
-                               gboolean focus_in)
-{
-  NuxBaseWindowAccessible* self = NULL;
-
-  /* On the base window, we suppose that the window is active if it
-     has the key focus (see nux::InputArea) */
-  self = NUX_BASE_WINDOW_ACCESSIBLE(object);
-
-  if (self->priv->key_focused != focus_in)
-  {
-    self->priv->key_focused = focus_in;
-  }
-}
-
-/* public */
-void
-nux_base_window_set_child_key_focused(NuxBaseWindowAccessible* self,
-                                      gboolean value)
-{
-  g_return_if_fail(NUX_IS_BASE_WINDOW_ACCESSIBLE(self));
-
-  if (self->priv->child_key_focused != value)
-  {
-    self->priv->child_key_focused = value;
-  }
-}
-
-void
-nux_base_window_accessible_check_active(NuxBaseWindowAccessible* self)
-{
-  g_return_if_fail(NUX_IS_BASE_WINDOW_ACCESSIBLE(self));
-
-  check_active(self);
+    g_signal_emit_by_name(self, signal_name, 0);
+  }
 }

=== modified file 'plugins/unityshell/src/nux-base-window-accessible.h'
--- plugins/unityshell/src/nux-base-window-accessible.h	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/nux-base-window-accessible.h	2011-11-07 22:02:27 +0000
@@ -54,11 +54,8 @@
 
 GType      nux_base_window_accessible_get_type(void);
 AtkObject* nux_base_window_accessible_new(nux::Object* object);
-
-void       nux_base_window_set_child_key_focused(NuxBaseWindowAccessible* self,
-                                                 gboolean value);
-
-void       nux_base_window_accessible_check_active(NuxBaseWindowAccessible* self);
+void       nux_base_window_accessible_check_active(NuxBaseWindowAccessible* self,
+                                                   nux::BaseWindow* active_window);
 
 
 G_END_DECLS

=== modified file 'plugins/unityshell/src/nux-view-accessible.cpp'
--- plugins/unityshell/src/nux-view-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/nux-view-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -260,21 +260,8 @@
 
   if (self->priv->key_focused != focus_in)
   {
-    AtkObject* parent_window = NULL;
-
     self->priv->key_focused = focus_in;
 
-    /* this child has the key focus, so we report the top level
-     * window about it. FIXME: that only works if only one object of
-     * the children hierarchy can have the key focus, that is the
-     * case here */
-
-    parent_window =
-      nux_area_accessible_get_parent_window(NUX_AREA_ACCESSIBLE(self));
-
-    nux_base_window_set_child_key_focused(NUX_BASE_WINDOW_ACCESSIBLE(parent_window),
-                                          focus_in);
-
     /* we always led the focus notification to
        _check_pending_notification, in order to allow the proper
        window_activate -> focus_change order */

=== modified file 'plugins/unityshell/src/unity-launcher-icon-accessible.cpp'
--- plugins/unityshell/src/unity-launcher-icon-accessible.cpp	2011-10-06 04:18:36 +0000
+++ plugins/unityshell/src/unity-launcher-icon-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -326,7 +326,6 @@
     atk_object_notify_state_change(ATK_OBJECT(self),
                                    ATK_STATE_ACTIVE,
                                    found);
-
     g_signal_emit_by_name(self, "focus-event", self->priv->selected, &return_val);
     atk_focus_tracker_notify(ATK_OBJECT(self));
   }

=== modified file 'plugins/unityshell/src/unity-quicklist-accessible.cpp'
--- plugins/unityshell/src/unity-quicklist-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/unity-quicklist-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -132,9 +132,6 @@
 
   if (quicklist == NULL) /* status defunct */
     return;
-
-  atk_object_set_name(accessible, "Quicklist");
-
 }
 
 static gint

=== modified file 'plugins/unityshell/src/unity-quicklist-menu-accessible.cpp'
--- plugins/unityshell/src/unity-quicklist-menu-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/unity-quicklist-menu-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -174,7 +174,7 @@
     return;
 
   atk_object_set_role(accessible, ATK_ROLE_MENU);
-  atk_object_set_name(accessible, "QuicklistMenu");
+  atk_object_set_name(accessible, _("Quicklist"));
 
   self->priv->on_selection_change_connection =
     quicklist->selection_change.connect(sigc::bind(sigc::ptr_fun(on_selection_change_cb), self));

=== modified file 'plugins/unityshell/src/unity-quicklist-menu-item-accessible.cpp'
--- plugins/unityshell/src/unity-quicklist-menu-item-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/unity-quicklist-menu-item-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -279,6 +279,9 @@
 
     self->priv->selected = found;
     atk_object_notify_state_change(ATK_OBJECT(self),
+                                   ATK_STATE_FOCUSED,
+                                   found);
+    atk_object_notify_state_change(ATK_OBJECT(self),
                                    ATK_STATE_SELECTED,
                                    found);
     atk_object_notify_state_change(ATK_OBJECT(self),

=== modified file 'plugins/unityshell/src/unity-root-accessible.cpp'
--- plugins/unityshell/src/unity-root-accessible.cpp	2011-11-07 22:02:27 +0000
+++ plugins/unityshell/src/unity-root-accessible.cpp	2011-11-07 22:02:27 +0000
@@ -21,7 +21,8 @@
  * @short_description: Root object for the UNITY accessible support
  *
  * #UnityRootAccessible is the root object of the accessibility
- * tree-like hierarchy, exposing the application level.
+ * tree-like hierarchy, exposing the application level. You can see it
+ * as the one exposing UnityScreen information to the a11y framework
  *
  */
 
@@ -63,6 +64,8 @@
   /* we save on window_list the accessible object for the windows
      registered */
   GSList* window_list;
+  nux::BaseWindow* active_window;
+  nux::BaseWindow* launcher_window;
 };
 
 static void
@@ -88,6 +91,8 @@
   root->priv = UNITY_ROOT_ACCESSIBLE_GET_PRIVATE(root);
 
   root->priv->window_list = NULL;
+  root->priv->active_window = NULL;
+  root->priv->launcher_window = NULL;
 }
 
 AtkObject*
@@ -231,8 +236,7 @@
   for (iter = self->priv->window_list; iter != NULL; iter = g_slist_next(iter))
   {
     window = NUX_BASE_WINDOW_ACCESSIBLE(iter->data);
-
-    nux_base_window_accessible_check_active(window);
+    nux_base_window_accessible_check_active(window, self->priv->active_window);
   }
 }
 
@@ -303,19 +307,78 @@
 }
 
 static void
-ubus_change_visibility_cb(GVariant* variant,
-                          UnityRootAccessible* self)
+set_active_window(UnityRootAccessible* self,
+                  nux::BaseWindow* window)
 {
+  g_return_if_fail(UNITY_IS_ROOT_ACCESSIBLE(self));
+  g_return_if_fail(window != NULL);
+
+  self->priv->active_window = window;
   check_active_window(self);
 }
 
-static void
-wc_change_visibility_window_cb(nux::BaseWindow* window, UnityRootAccessible* self, gboolean visible)
+nux::BaseWindow*
+search_for_launcher_window(UnityRootAccessible* self)
+{
+  GSList*iter = NULL;
+  nux::Object* nux_object = NULL;
+  nux::BaseWindow* bwindow = NULL;
+  NuxObjectAccessible* accessible = NULL;
+  gboolean found = FALSE;
+
+  for (iter = self->priv->window_list; iter != NULL; iter = g_slist_next(iter))
+  {
+    accessible = NUX_OBJECT_ACCESSIBLE(iter->data);
+
+    nux_object = nux_object_accessible_get_object(accessible);
+    bwindow = dynamic_cast<nux::BaseWindow*>(nux_object);
+
+    if ((bwindow!= NULL) && (g_strcmp0(bwindow->GetWindowName().GetTCharPtr(), "Launcher") == 0))
+    {
+      found = TRUE;
+      break;
+    }
+  }
+
+  if (found)
+    return bwindow;
+  else
+    return NULL;
+}
+
+static void
+ubus_launcher_start_key_nav_cb(GVariant* variant,
+                               UnityRootAccessible* self)
+{
+  //launcher window is the same during all the life of Unity
+  if (self->priv->launcher_window == NULL)
+    self->priv->launcher_window = search_for_launcher_window(self);
+
+  //launcher window became the active window
+  set_active_window(self, self->priv->launcher_window);
+}
+
+
+static void
+wc_change_visibility_window_cb(nux::BaseWindow* window,
+                               UnityRootAccessible* self,
+                               gboolean visible)
 {
   if (visible)
+  {
     add_window(self, window);
+    //for the dash and quicklist
+    set_active_window(self, window);
+  }
   else
+  {
+    AtkObject* accessible = NULL;
+
+    accessible = unity_a11y_get_accessible(window);
+    nux_base_window_accessible_check_active(NUX_BASE_WINDOW_ACCESSIBLE(accessible),
+                                            NULL);
     remove_window(self, window);
+  }
 }
 
 static void
@@ -323,15 +386,8 @@
 {
   static unity::UBusManager ubus_manager;
 
-  ubus_manager.RegisterInterest(UBUS_PLACE_VIEW_SHOWN,
-                                sigc::bind(sigc::ptr_fun(ubus_change_visibility_cb), self));
-  ubus_manager.RegisterInterest(UBUS_PLACE_VIEW_HIDDEN,
-                                sigc::bind(sigc::ptr_fun(ubus_change_visibility_cb), self));
-
   ubus_manager.RegisterInterest(UBUS_LAUNCHER_START_KEY_NAV,
-                                sigc::bind(sigc::ptr_fun(ubus_change_visibility_cb), self));
-  ubus_manager.RegisterInterest(UBUS_LAUNCHER_END_KEY_NAV,
-                                sigc::bind(sigc::ptr_fun(ubus_change_visibility_cb), self));
+                                sigc::bind(sigc::ptr_fun(ubus_launcher_start_key_nav_cb), self));
 
   nux::GetWindowCompositor().sigVisibleViewWindow.
   connect(sigc::bind(sigc::ptr_fun(wc_change_visibility_window_cb), self, TRUE));