Merge lp:~autopilot/autopilot-gtk/experimental into lp:autopilot-gtk

[Thomi Richards]

Version 1.4 wire protocol changes.

Version 1.4 protocol changes.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:64
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-ci/61/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-ci/23/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-gtk-ci/61/rebuild

review: Needs Fixing (continuous-integration)

[Martin Pitt (pitti)]

This mostly looks okay visually, except that this needs to grow a

  Breaks: autopilot (<< 1.4)

to avoid trying to install an 1.4 version of libautopilot-gtk with an 1.3 version of autopilot.

Also, actually approving this branch needs to wait for the PS jenkins tests to succeed, which requires landing xpathselect first.

review: Needs Fixing

[Martin Pitt (pitti)]

Err, of course not "Breaks: autpilot", but "Breaks: python-autopilot (<< 1.4), python3-autopilot (<< 1.4)"

The latter is not strictly necessary as we did'nt yet have p3-ap for 1.3, but it'll provide a nice template for bumping to 1.5 in the future.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:65
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-ci/62/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-ci/24/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-ci/24/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-gtk-ci/62/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-autolanding/17/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-autolanding/11/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-autolanding/11/console

review: Needs Fixing (continuous-integration)

[Francis Ginther (fginther)]

Autopilot is now in the local archive, re-approve.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-autolanding/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-autolanding/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-autolanding/12
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-i386-autolanding/1/console
    SUCCESS: http://s-jenkins:8080/job/generic-cleanup-mbs/2232

review: Needs Fixing (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

Above i386 failure was xvfb failing to start, trying again, since it seems like an intermittent failure.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-autolanding/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-autolanding/13
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-autolanding/13
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-i386-autolanding/2/console
    SUCCESS: http://s-jenkins:8080/job/generic-cleanup-mbs/2240

review: Needs Fixing (continuous-integration)

[Martin Pitt (pitti)]

The requested Breaks: got added, so changing my vote to approved.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:68
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-ci/63/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-ci/25
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-ci/25/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-i386-ci/1

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-gtk-ci/63/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-autolanding/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-autolanding/14
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-autolanding/14/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-i386-autolanding/3

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'debian/changelog'
--- debian/changelog	2013-06-28 00:01:10 +0000
+++ debian/changelog	2013-09-17 13:46:39 +0000
@@ -1,3 +1,10 @@
+autopilot-gtk (1.4-0ubuntu1) saucy; urgency=low
+
+  [ Thomi Richards ]
+  * Version bump, updatedx to new xpathselect API.
+
+ -- Thomi Richards <[e-mail]>  Thu, 15 Aug 2013 15:12:39 +1200
+
 autopilot-gtk (1.3+13.10.20130628-0ubuntu1) saucy; urgency=low
 
   [ Martin Pitt ]

=== modified file 'debian/control'
--- debian/control	2013-06-25 15:35:28 +0000
+++ debian/control	2013-09-17 13:46:39 +0000
@@ -7,14 +7,13 @@
                pkg-config,
                libglib2.0-dev,
                libgtk-3-dev,
-               libxpathselect-dev (>= 1.3),
+               libxpathselect-dev (>= 1.4),
                xvfb,
                dbus-x11,
-               python-autopilot,
-               python-xlib,
+               autopilot-desktop,
+               bamfdaemon,
                python-evdev,
                python-gi,
-               gir1.2-gtk-3.0,
                gsettings-desktop-schemas,
                gnome-system-log,
 Standards-Version: 3.9.4
@@ -33,6 +32,7 @@
 Pre-Depends: ${misc:Pre-Depends},
 Depends: ${shlibs:Depends},
          ${misc:Depends},
+Breaks: python-autopilot (<< 1.4), python3-autopilot (<< 1.4)
 Replaces: autopilot-gtk, libautopilot-gtk0, libautopilot-gtk-dev
 Conflicts: autopilot-gtk, libautopilot-gtk0, libautopilot-gtk-dev
 Provides: autopilot-gtk, libautopilot-gtk0

=== modified file 'debian/rules'
--- debian/rules	2013-06-26 06:05:38 +0000
+++ debian/rules	2013-09-17 13:46:39 +0000
@@ -9,7 +9,8 @@
 override_dh_auto_test:
 	mkdir -p debian/tmp/home/run
 	env HOME=$(CURDIR)/debian/tmp/home XDG_RUNTIME_DIR=$(CURDIR)/debian/tmp/home/run \
-		xvfb-run dh_auto_test
+		xvfb-run -a -e $(CURDIR)/debian/tmp/X.log dh_auto_test || \
+	        { echo "==== X.org log ===="; cat $(CURDIR)/debian/tmp/X.log; false; }
 
 override_dh_install:
 	dh_install --fail-missing

=== modified file 'lib/GtkNode.cpp'
--- lib/GtkNode.cpp	2013-06-27 17:02:58 +0000
+++ lib/GtkNode.cpp	2013-09-17 13:46:39 +0000
@@ -25,11 +25,40 @@
 #include "Variant.h"
 
 const std::string GtkNode::AP_ID_NAME = "id";
+static guint32 cur_obj_id = 2; // start at 2 since 1 is reserved for the root node
 
-GtkNode::GtkNode(GObject* obj, std::string const& parent_path)
-  : object_(obj) {
+GtkNode::GtkNode(GObject* obj, GtkNode::Ptr const& parent)
+  : object_(obj)
+  , parent_(parent)
+{
+  std::string parent_path = parent ? parent->GetPath() : "";
   full_path_ = parent_path + "/" + GetName();
-  if (object_ != NULL) g_object_ref(object_);
+  if (object_ != NULL)
+  {
+    g_object_ref(object_);
+    GQuark OBJ_ID = g_quark_from_static_string("AUTOPILOT_OBJECT_ID");
+    gpointer val = g_object_get_qdata (object_, OBJ_ID);
+    if (val == NULL)
+    {
+      g_object_set_qdata (object_, OBJ_ID, reinterpret_cast<gpointer>(cur_obj_id++));
+    }
+  }
+}
+
+GtkNode::GtkNode(GObject* obj)
+  : object_(obj)
+{
+  full_path_ = "/" + GetName();
+  if (object_ != NULL)
+  {
+    g_object_ref(object_);
+    GQuark OBJ_ID = g_quark_from_static_string("AUTOPILOT_OBJECT_ID");
+    gpointer val = g_object_get_qdata (object_, OBJ_ID);
+    if (val == NULL)
+    {
+      g_object_set_qdata (object_, OBJ_ID, reinterpret_cast<gpointer>(cur_obj_id++));
+    }
+  }
 }
 
 GtkNode::~GtkNode()
@@ -109,7 +138,7 @@
         g_value_init(&value, param_spec->value_type);
         g_object_get_property(object_, g_param_spec_get_name(param_spec), &value);
         convert_value(param_spec, &value);
-        builder_wrapper.add(param_spec->name, &value);
+        builder_wrapper.add_gvalue(param_spec->name, &value);
         g_value_unset(&value); //Free the memory accquired by the value object. Absence of this was causig the applications to crash.
       }
     } else {
@@ -119,7 +148,7 @@
   g_free(properties);
 
   // add our unique autopilot-id
-  builder_wrapper.add(AP_ID_NAME.c_str(), GetObjectId());
+  builder_wrapper.add(AP_ID_NAME.c_str(), GetId());
 
   // add the names of our children
   builder_wrapper.add("Children", GetChildNodeNames());
@@ -136,9 +165,7 @@
     if (GDK_IS_WINDOW(gdk_window)) {
       GdkRectangle rect;
       GetGlobalRect(&rect);
-      //g_debug("Rect coords %d, %d, %d, %d", rect.x, rect.y, rect.width, rect.height);
-      GVariant *rect_gvariant = ComposeRectVariant(rect.x, rect.y, rect.width, rect.height);
-      builder_wrapper.add("globalRect", rect_gvariant);
+      builder_wrapper.add("globalRect", rect);
     }
   } else if (ATK_IS_COMPONENT(object_)) {
     AddAtkComponentProperties(builder_wrapper, ATK_COMPONENT(object_));
@@ -146,18 +173,6 @@
   return g_variant_builder_end(&builder);
 }
 
-GVariant* GtkNode::ComposeRectVariant(gint x, gint y, gint height, gint width) const
-{
-  //g_debug("composing a rect variant");
-  GVariantBuilder builder;
-  g_variant_builder_init(&builder, G_VARIANT_TYPE_ARRAY);
-  g_variant_builder_add(&builder, "i", x);
-  g_variant_builder_add(&builder, "i", y);
-  g_variant_builder_add(&builder, "i", height);
-  g_variant_builder_add(&builder, "i", width);
-  return g_variant_builder_end(&builder);
-}
-
 void GtkNode::AddAtkComponentProperties(variant::BuilderWrapper &builder_wrapper,
                                         AtkComponent *atk_component) const
 {
@@ -171,8 +186,12 @@
     x = y = width = height = -1;
     atk_component_get_extents(atk_component, &x, &y, &width, &height,
                               ATK_XY_SCREEN);
-    GVariant *rect_gvariant = ComposeRectVariant(x, y, width, height);
-    builder_wrapper.add("globalRect", rect_gvariant);
+    GdkRectangle r;
+    r.x = x;
+    r.y = y;
+    r.width = width;
+    r.height = height;
+    builder_wrapper.add("globalRect", r);
   }
 
   builder_wrapper.add("active",
@@ -196,10 +215,6 @@
   g_object_unref(G_OBJECT(states));
 }
 
-intptr_t GtkNode::GetObjectId() const {
-  return reinterpret_cast<intptr_t>(object_);
-}
-
 void GtkNode::GetGlobalRect(GdkRectangle* rect) const
 {
   GtkWidget *widget = GTK_WIDGET(object_);
@@ -233,11 +248,24 @@
   return full_path_;
 }
 
-bool GtkNode::MatchProperty(const std::string& name,
-                            const std::string& value) const {
-  if (name == "id")
-    return value == std::to_string(GetObjectId());
+int32_t GtkNode::GetId() const
+{
+  GQuark OBJ_ID = g_quark_from_static_string("AUTOPILOT_OBJECT_ID");
+  gpointer val = g_object_get_qdata (object_, OBJ_ID);
+  // this uglyness is required in order to stop the compiler complaining about the fact
+  // that we're casting a 64 bit type (gpointer) down to a 32 bit type (gint32) and may
+  // be truncating the value. It's safe to do, however, since we control what values are
+  // set in this quark, and they were initial gint32 values anyway.
+  guint32 id = static_cast<gint32>(reinterpret_cast<intptr_t>(val));
+  return id;
+}
 
+xpathselect::Node::Ptr GtkNode::GetParent() const
+{
+  return parent_;
+}
+bool GtkNode::MatchStringProperty(const std::string& name,
+                                  const std::string& value) const {
   if (name == "BuilderName" && GTK_IS_BUILDABLE(object_)) {
       const gchar* name = gtk_buildable_get_name(GTK_BUILDABLE (object_));
       return name != NULL && std::string(name) == value;
@@ -258,41 +286,100 @@
   convert_value(pspec, &dest_value);
   std::string dest_string;
 
-  // convert it to a string; always doing string comparison avoids having to do
-  // type comparison, conversion and error handling on value
-  switch (G_VALUE_TYPE(&dest_value)) {
-    case G_TYPE_INT:
-      dest_string = std::to_string(g_value_get_int(&dest_value));
-      break;
-    case G_TYPE_UINT:
-      dest_string = std::to_string(g_value_get_uint(&dest_value));
-      break;
-    case G_TYPE_DOUBLE:
-      dest_string = std::to_string(g_value_get_double(&dest_value));
-      break;
-    case G_TYPE_STRING: {
+  if (G_VALUE_TYPE(&dest_value) == G_TYPE_STRING) {
       const gchar *str = g_value_get_string(&dest_value);
       dest_string = (str != NULL) ? str : "";
-      break;
-      }
-    default:
-      g_debug("Unhandled type %s for matching property %s",
-              g_type_name(G_VALUE_TYPE(&dest_value)), g_param_spec_get_name(pspec));
-      g_value_unset(&dest_value);
-      return false;
-  }
-
-  g_value_unset(&dest_value);
-  return dest_string == value;
-}
-
-xpathselect::NodeList GtkNode::Children() const {
+      g_value_unset(&dest_value);
+      return dest_string == value;
+  }
+  else {
+      g_debug("Property %s exists, but is not a string (is %s).",
+              g_param_spec_get_name(pspec),
+              g_type_name(G_VALUE_TYPE(&dest_value))
+              );
+      g_value_unset(&dest_value);
+      return false;
+    }
+
+}
+
+bool GtkNode::MatchIntegerProperty(const std::string& name,
+                                  int32_t value) const {
+  if (name == "id")
+    return value == GetId();
+
+  GObjectClass* klass = G_OBJECT_GET_CLASS(object_);
+  GParamSpec* pspec = g_object_class_find_property(klass, name.c_str());
+  if (pspec == NULL)
+    return false;
+
+  // read the property into a GValue
+  g_debug("Matching property %s of type (%s).", g_param_spec_get_name(pspec),
+          g_type_name(G_PARAM_SPEC_VALUE_TYPE(pspec)));
+
+  GValue dest_value = G_VALUE_INIT;
+  g_value_init(&dest_value, G_PARAM_SPEC_VALUE_TYPE(pspec));
+  g_object_get_property(object_, name.c_str(), &dest_value);
+  convert_value(pspec, &dest_value);
+
+  if (G_VALUE_TYPE(&dest_value) == G_TYPE_INT) {
+    int v = g_value_get_int(&dest_value);
+    g_value_unset(&dest_value);
+    return value == v;
+  }
+  else if (G_VALUE_TYPE(&dest_value) == G_TYPE_UINT) {
+    int v = g_value_get_uint(&dest_value);
+    g_value_unset(&dest_value);
+    return value == v;
+  }
+  else {
+      g_debug("Property %s exists, but is not an integer (is %s).",
+              g_param_spec_get_name(pspec),
+              g_type_name(G_VALUE_TYPE(&dest_value))
+              );
+      g_value_unset(&dest_value);
+      return false;
+    }
+}
+
+bool GtkNode::MatchBooleanProperty(const std::string& name,
+                                   bool value) const {
+  GObjectClass* klass = G_OBJECT_GET_CLASS(object_);
+  GParamSpec* pspec = g_object_class_find_property(klass, name.c_str());
+  if (pspec == NULL)
+    return false;
+
+  // read the property into a GValue
+  g_debug("Matching property %s of type (%s).", g_param_spec_get_name(pspec),
+          g_type_name(G_PARAM_SPEC_VALUE_TYPE(pspec)));
+
+  GValue dest_value = G_VALUE_INIT;
+  g_value_init(&dest_value, G_PARAM_SPEC_VALUE_TYPE(pspec));
+  g_object_get_property(object_, name.c_str(), &dest_value);
+  convert_value(pspec, &dest_value);
+
+  if (G_VALUE_TYPE(&dest_value) == G_TYPE_BOOLEAN) {
+    bool v = g_value_get_boolean(&dest_value);
+    g_value_unset(&dest_value);
+    return value == v;
+  }
+  else {
+      g_debug("Property %s exists, but is not a boolean (is %s).",
+              g_param_spec_get_name(pspec),
+              g_type_name(G_VALUE_TYPE(&dest_value))
+              );
+      g_value_unset(&dest_value);
+      return false;
+    }
+}
+
+xpathselect::NodeVector GtkNode::Children() const {
   //g_debug("getting the children of a node");
-  xpathselect::NodeList children;
+  xpathselect::NodeVector children;
   if (GTK_IS_CONTAINER(object_)) {
     GList* gtk_children = gtk_container_get_children(GTK_CONTAINER(object_));
     for (GList* elem = gtk_children; elem; elem = elem->next) {
-      children.push_back(std::make_shared<GtkNode>(G_OBJECT(elem->data), GetPath()));
+      children.push_back(std::make_shared<GtkNode>(G_OBJECT(elem->data), shared_from_this()));
     }
     g_list_free(gtk_children);
   } else if (ATK_IS_OBJECT(object_)) {
@@ -300,7 +387,7 @@
     int n_children = atk_object_get_n_accessible_children(atk_object);
     for (int i = 0; i < n_children; i++) {
       AtkObject *child = atk_object_ref_accessible_child(atk_object, i);
-      children.push_back(std::make_shared<GtkNode>(G_OBJECT(child), GetPath()));
+      children.push_back(std::make_shared<GtkNode>(G_OBJECT(child), shared_from_this()));
     }
   }
 

=== modified file 'lib/GtkNode.h'
--- lib/GtkNode.h	2013-04-21 23:04:43 +0000
+++ lib/GtkNode.h	2013-09-17 13:46:39 +0000
@@ -25,33 +25,43 @@
 #include <xpathselect/node.h>
 #include <xpathselect/xpathselect.h>
 #include <string>
+#include <cstdint>
 #include "Variant.h"
 
-class GtkNode: public xpathselect::Node {
+// #include <memory>
+
+class GtkNode: public xpathselect::Node, public std::enable_shared_from_this<GtkNode>
+{
 public:
-  typedef std::shared_ptr<GtkNode> Ptr;
+  typedef std::shared_ptr<const GtkNode> Ptr;
 
-  GtkNode(GObject* object, std::string const& parent_name);
+  GtkNode(GObject* object, Ptr const& parent);
+  GtkNode(GObject* object);
   virtual ~GtkNode();
 
   virtual GVariant* Introspect() const;
 
   virtual std::string GetName() const;
   virtual std::string GetPath() const;
-  virtual bool MatchProperty(const std::string& name,
-                             const std::string& value) const;
-  virtual xpathselect::NodeList Children() const;
+  virtual int32_t GetId() const;
+  virtual xpathselect::Node::Ptr GetParent() const;
+  virtual bool MatchStringProperty(const std::string& name,
+                                   const std::string& value) const;
+  virtual bool MatchIntegerProperty(const std::string& name,
+                                    int32_t value) const;
+  virtual bool MatchBooleanProperty(const std::string& name,
+                                    bool value) const;
+  virtual xpathselect::NodeVector Children() const;
 
   static const std::string AP_ID_NAME;
 
 private:
   GObject *object_;
   std::string full_path_;
+  Ptr parent_;
 
   virtual GVariant* GetChildNodeNames() const;
-  virtual intptr_t GetObjectId() const;
   virtual void GetGlobalRect(GdkRectangle* rect) const;
-  virtual GVariant* ComposeRectVariant(gint x, gint y, gint height, gint width) const;
   void AddAtkComponentProperties(variant::BuilderWrapper &builder_wrapper,
                                  AtkComponent *atk_component) const;
 };

=== modified file 'lib/GtkRootNode.cpp'
--- lib/GtkRootNode.cpp	2013-04-21 23:04:43 +0000
+++ lib/GtkRootNode.cpp	2013-09-17 13:46:39 +0000
@@ -27,7 +27,7 @@
 #include "Variant.h"
 
 GtkRootNode::GtkRootNode()
-  : GtkNode(NULL, std::string()) {
+  : GtkNode(NULL) {
 }
 
 GVariant* GtkRootNode::Introspect() const {
@@ -35,14 +35,13 @@
   g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}"));
   variant::BuilderWrapper builder_wrapper(&builder);
   // add our unique autopilot-id
-  builder_wrapper.add(AP_ID_NAME.c_str(), GetObjectId());
+  builder_wrapper.add(AP_ID_NAME.c_str(), GetId());
   // add the names of our children
   builder_wrapper.add("Children", GetChildNodeNames());
   return g_variant_builder_end(&builder);
 }
 
-intptr_t GtkRootNode::GetObjectId() const {
-  // FIXME: we could be consistent and return the memory address
+int32_t GtkRootNode::GetId() const {
   return 1;
 }
 
@@ -54,30 +53,39 @@
   return "/" + GetName();
 }
 
-bool GtkRootNode::MatchProperty(const std::string& name,
-                                const std::string& value) const {
-  //g_debug("matching a property for the root node");
+bool GtkRootNode::MatchIntegerProperty(const std::string& name, int32_t value) const
+{
+  // Root node only matches one property - id:
   if (name == "id")
-    // yes we just compare as a string; use of intptr_t makes casting tricky
-    return value == std::to_string(GetObjectId());
-  return false;
-}
-
-xpathselect::NodeList GtkRootNode::Children() const {
+    return value == GetId();
+  return false;
+}
+
+bool GtkRootNode::MatchBooleanProperty(const std::string& name, bool value) const
+{
+  return false;
+}
+
+bool GtkRootNode::MatchStringProperty(const std::string& name, const std::string& value) const
+{
+  return false;
+}
+
+xpathselect::NodeVector GtkRootNode::Children() const {
   //g_debug("getting the children of a node");
-  xpathselect::NodeList children;
+  xpathselect::NodeVector children;
 
   // add all the toplevel nodes as children to the root node
   GList* toplevels_list = gtk_window_list_toplevels();
   GList* elem;
   for (elem = toplevels_list; elem; elem = elem->next) {
     GObject *node = reinterpret_cast<GObject*>(elem->data);
-    children.push_back(std::make_shared<GtkNode>(node, GetPath()));
+    children.push_back(std::make_shared<GtkNode>(node, shared_from_this()));
 
     // if the AtkObjects are available, expose the Atk hierarchy as well
     AtkObject *atk_object = gtk_widget_get_accessible(GTK_WIDGET(node));
     if (atk_object != NULL)
-        children.push_back(std::make_shared<GtkNode>(G_OBJECT(atk_object), GetPath()));
+        children.push_back(std::make_shared<GtkNode>(G_OBJECT(atk_object), shared_from_this()));
   }
   g_list_free(toplevels_list);
   return children;

=== modified file 'lib/GtkRootNode.h'
--- lib/GtkRootNode.h	2013-04-18 05:10:07 +0000
+++ lib/GtkRootNode.h	2013-09-17 13:46:39 +0000
@@ -33,12 +33,14 @@
   GtkRootNode();
 
   virtual GVariant* Introspect() const;
-  virtual intptr_t GetObjectId() const;
 
   virtual std::string GetName() const;
   virtual std::string GetPath() const;
-  virtual bool MatchProperty(const std::string& name, const std::string& value) const;
-  virtual xpathselect::NodeList Children() const;
+  virtual int32_t GetId() const;
+  virtual bool MatchIntegerProperty(const std::string& name, int32_t value) const;
+  virtual bool MatchBooleanProperty(const std::string& name, bool value) const;
+  virtual bool MatchStringProperty(const std::string& name, const std::string& value) const;
+  virtual xpathselect::NodeVector Children() const;
 
  private:
   virtual GVariant* GetChildNodeNames() const;

=== modified file 'lib/Introspection.cpp'
--- lib/Introspection.cpp	2013-05-08 02:52:12 +0000
+++ lib/Introspection.cpp	2013-09-17 13:46:39 +0000
@@ -82,7 +82,7 @@
 {
     autopilot_introspection_complete_get_version(introspection_service,
                                                  invocation,
-                                                 "1.3");
+                                                 "1.4");
     return TRUE;
 }
 
@@ -110,12 +110,12 @@
 
   //g_debug("selecting nodes");
   std::list<GtkNode::Ptr> node_list;
-  xpathselect::NodeList selected_nodes_list;
+  xpathselect::NodeVector selected_nodes_list;
   selected_nodes_list = xpathselect::SelectNodes(root, query_string);
   //g_debug("finished selecting nodes");
   for (auto node : selected_nodes_list) {
     // node may be our root node wrapper *or* an ordinary GObject wrapper
-    auto object_ptr = std::static_pointer_cast<GtkNode>(node);
+    auto object_ptr = std::static_pointer_cast<const GtkNode>(node);
     if (object_ptr)
       node_list.push_back(object_ptr);
   }

=== modified file 'lib/Variant.cpp'
--- lib/Variant.cpp	2013-02-13 02:38:30 +0000
+++ lib/Variant.cpp	2013-09-17 13:46:39 +0000
@@ -17,9 +17,12 @@
  * Authored by: Tim Penhey <[e-mail]>
  */
 
+#include <gdk/gdk.h>
+
 #include <iostream>
 #include "Variant.h"
 
+#include "autopilot_types.h"
 namespace variant
 {
 
@@ -29,82 +32,210 @@
 
 BuilderWrapper& BuilderWrapper::add(char const* name, bool value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_boolean(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_boolean(value));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, char const* value)
 {
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
   if (value)
-    g_variant_builder_add(builder_, "{sv}", name, g_variant_new_string(value));
+    g_variant_builder_add(&b, "v", g_variant_new_string(value));
   else
-    g_variant_builder_add(builder_, "{sv}", name, g_variant_new_string(""));
+    g_variant_builder_add(&b, "v", g_variant_new_string(""));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
 
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, std::string const& value)
 {
-  g_variant_builder_add(builder_, "{sv}", name,
-                        g_variant_new_string(value.c_str()));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_string(value.c_str()));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_int32(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_int32(value));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, long int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_int64(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_int64(value));
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, long long int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_int64(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_int64(value));
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, unsigned int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_uint32(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_uint32(value));
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, long unsigned int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_uint64(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_uint64(value));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, long long unsigned int value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_uint64(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_uint64(value));
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, float value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_double(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_double(value));
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, double value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, g_variant_new_double(value));
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+  g_variant_builder_add(&b, "v", g_variant_new_double(value));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
   return *this;
 }
 
 BuilderWrapper& BuilderWrapper::add(char const* name, GVariant* value)
 {
-  g_variant_builder_add(builder_, "{sv}", name, value);
+  if (value)
+  {
+    GVariantBuilder b;
+    g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+    g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_PLAIN));
+    g_variant_builder_add(&b, "v", value);
+    g_variant_builder_add(
+      builder_,
+      "{sv}",
+      name,
+      g_variant_builder_end(&b)
+    );
+  }
   return *this;
 }
 
-BuilderWrapper& BuilderWrapper::add(char const* name, GValue* value)
+BuilderWrapper& BuilderWrapper::add_gvalue(char const* name, GValue* value)
 {
   switch (G_VALUE_TYPE(value)) {
   case G_TYPE_CHAR:
@@ -119,7 +250,7 @@
     break;
   case G_TYPE_BOOLEAN:
     {
-      add(name, g_value_get_boolean(value));
+      add(name, (bool) g_value_get_boolean(value));
     }
     break;
   case G_TYPE_INT:
@@ -197,17 +328,32 @@
       add(name, g_value_get_object(value));
     }
     break;
-  case G_TYPE_VARIANT:
-    {
-      add(name, g_value_get_variant(value));
-    }
-    break;
   default:
-    //g_warning("unsupported type: %s", g_type_name(G_VALUE_TYPE(value)));
+    g_warning("unsupported type: %s", g_type_name(G_VALUE_TYPE(value)));
     {}
     break;
   }
   return *this;
 }
 
+BuilderWrapper& BuilderWrapper::BuilderWrapper::add(char const* name, GdkRectangle value)
+{
+  GVariantBuilder b;
+  g_variant_builder_init(&b, G_VARIANT_TYPE("av"));
+
+  g_variant_builder_add(&b, "v", g_variant_new_int32(TYPE_RECT));
+  g_variant_builder_add(&b, "v", g_variant_new_int32(value.x));
+  g_variant_builder_add(&b, "v", g_variant_new_int32(value.y));
+  g_variant_builder_add(&b, "v", g_variant_new_int32(value.width));
+  g_variant_builder_add(&b, "v", g_variant_new_int32(value.height));
+
+  g_variant_builder_add(
+    builder_,
+    "{sv}",
+    name,
+    g_variant_builder_end(&b)
+  );
+  return *this;
+}
+
 }

=== modified file 'lib/Variant.h'
--- lib/Variant.h	2013-02-13 02:38:30 +0000
+++ lib/Variant.h	2013-09-17 13:46:39 +0000
@@ -46,7 +46,9 @@
   BuilderWrapper& add(char const* name, float value);
   BuilderWrapper& add(char const* name, double value);
   BuilderWrapper& add(char const* name, GVariant* value);
-  BuilderWrapper& add(char const* name, GValue* value);
+  BuilderWrapper& add_gvalue(char const* name, GValue* value);
+
+  BuilderWrapper& add(char const* name, GdkRectangle value);
 
 private:
   GVariantBuilder* builder_;

=== added file 'lib/autopilot_types.h'
--- lib/autopilot_types.h	1970-01-01 00:00:00 +0000
+++ lib/autopilot_types.h	2013-09-17 13:46:39 +0000
@@ -0,0 +1,34 @@
+/* -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+ * Copyright (C) 2013 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 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, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef AUTOPILOT_TYPES_H
+#define AUTOPILOT_TYPES_H
+
+/// IMPORTANT: THese constants are taken from the autopilot XPathSelect protocol document.
+/// Only add options here if the support has been added for them in autopilot itself.
+enum autopilot_type_id
+{
+    TYPE_PLAIN = 0,
+    TYPE_RECT = 1,
+    TYPE_POINT = 2,
+    TYPE_SIZE = 3,
+    TYPE_COLOR = 4,
+    TYPE_DATETIME = 5,
+    TYPE_TIME = 6,
+};
+
+#endif

=== modified file 'tests/autopilot/tests/test_widget_tree.py'
--- tests/autopilot/tests/test_widget_tree.py	2013-06-26 09:31:14 +0000
+++ tests/autopilot/tests/test_widget_tree.py	2013-09-17 13:46:39 +0000
@@ -79,7 +79,7 @@
 
         # qualified: visible property is not unique
         self.assertRaises(ValueError,
-                          self.app.select_single, 'GtkButton', visible=1)
+                          self.app.select_single, 'GtkButton', visible=True)
 
         # qualified: label property is unique within GtkButton
         w = self.app.select_single('GtkButton', label='gtk-quit')
@@ -98,7 +98,7 @@
         self.assertRaises(ValueError, self.app.select_single, label='gtk-quit')
 
         # ... but it is unique for focussable widgets (menus don't allow that)
-        w = self.app.select_single(label='gtk-quit', can_focus=1)
+        w = self.app.select_single(label='gtk-quit', can_focus=True)
         self.assertIn('.GtkButton', str(type(w)))
         self.assertEqual(w.label, 'gtk-quit')
 
@@ -141,8 +141,6 @@
         # button and menu item
         self.assertEqual(len(res), 2)
 
-    # https://launchpad.net/bugs/1194763
-    @unittest.expectedFailure
     def test_select_int(self):
         """select_*() with int properties"""
 
@@ -158,8 +156,6 @@
 
         self.assertNotEqual(self.app.select_single(border_width=2), None)
 
-    # https://launchpad.net/bugs/1194763
-    @unittest.expectedFailure
     def test_select_bool(self):
         """select_*() with boolean properties"""
 

=== modified file 'tests/autopilot/tests/test_xpath_query.py'
--- tests/autopilot/tests/test_xpath_query.py	2013-06-25 12:58:00 +0000
+++ tests/autopilot/tests/test_xpath_query.py	2013-09-17 13:46:39 +0000
@@ -83,23 +83,22 @@
     def test_select_by_attribute(self):
         """Select widgets with attribute pattern"""
 
-        state = self.app.get_state_by_path('//*[label=gtk-delete]')
+        state = self.app.get_state_by_path('//*[label="gtk-delete"]')
         self.assertEqual(len(state), 1, state)
-        self.assertEqual(state[0][1]['label'], 'gtk-delete')
+        self.assertEqual(state[0][1]['label'], [0, 'gtk-delete'])
         self.assertTrue(state[0][0].endswith('/GtkButton'), state[0][0])
 
     # https://launchpad.net/bugs/1179806
+    # TODO: Make this pass!
     @unittest.expectedFailure
     def test_select_by_attribute_spaces(self):
         """Select widgets with attribute pattern containing spaces"""
 
-        # none of these work ATM, but are supposed to:
-        #state = self.app.get_state_by_path('//*[label=Hello&#x20;Color!]')
-        #state = self.app.get_state_by_path('//*[label=Hello&#x0020;Color!]')
-        state = self.app.get_state_by_path('//*[label="Hello Color!"]')
-        self.assertEqual(len(state), 1, str(state))
-        self.assertEqual(state[0][1]['label'], 'Hello Color!')
-        self.assertTrue(state[0][0].endswith('/GtkLabel'), state[0][0])
+        for state_str in ('//*[label="Hello\\x20Color!"]', '//*[label="Hello Color!"]'):
+            state = self.app.get_state_by_path(state_str)
+            self.assertEqual(len(state), 1, str(state))
+            self.assertEqual(state[0][1]['label'], 'Hello Color!')
+            self.assertTrue(state[0][0].endswith('/GtkLabel'), state[0][0])
 
     @classmethod
     def _get_widgets(klass, obj, widget_set):