diff --git a/xdg-shell.xml b/xdg-shell.xml
index 8c9804f..993c54a 100644
--- a/xdg-shell.xml
+++ b/xdg-shell.xml
@@ -608,6 +608,8 @@
     <enum name="error">
       <entry name="invalid_resize_edge" value="0" summary="provided value is
         not a valid variant of the resize_edge enum"/>
+      <entry name="invalid_parent" value="1"
+        summary="invalid parent toplevel"/>
     </enum>
 
     <request name="set_parent">
@@ -628,6 +630,10 @@
 	the now-unmapped surface. If the now-unmapped surface has no parent,
 	its children's parent is unset. If the now-unmapped surface becomes
 	mapped again, its parent-child relationship is not restored.
+
+	The parent toplevel must not be one of the child toplevel's
+	descendants, and the parent must be different from the child toplevel,
+	otherwise the invalid_parent protocol error is raised.
       </description>
       <arg name="parent" type="object" interface="xdg_toplevel" allow-null="true"/>
     </request>
diff --git a/xdg_popup.c b/xdg_popup.c
index be1a411..e6be419 100644
--- a/xdg_popup.c
+++ b/xdg_popup.c
@@ -350,12 +350,7 @@ Commit (Role *role, Surface *surface, XdgRoleImplementation *impl)
   popup = PopupFromRoleImpl (impl);
 
   if (popup->state & StatePendingPosition)
-    {
-      popup->x = popup->pending_x;
-      popup->y = popup->pending_y;
-
-      MoveWindow (popup);
-    }
+    MoveWindow (popup);
 
   popup->state &= ~StatePendingPosition;
 
@@ -395,7 +390,11 @@ AckConfigure (Role *role, XdgRoleImplementation *impl, uint32_t serial)
       popup->x = popup->pending_x;
       popup->y = popup->pending_y;
 
+      /* The position has been acked.  Clear that flag.  */
       popup->state &= ~StateAckPosition;
+
+      /* Set a new flag which tells commit to move the popup.  */
+      popup->state |= StatePendingPosition;
       popup->position_serial = 0;
     }
 }
diff --git a/xdg_toplevel.c b/xdg_toplevel.c
index e58111e..07b37f2 100644
--- a/xdg_toplevel.c
+++ b/xdg_toplevel.c
@@ -1520,6 +1520,21 @@ SetParent (struct wl_client *client, struct wl_resource *resource,
     UpdateParent (child, NULL);
   else
     UpdateParent (child, parent);
+
+  /* Now, verify that no circular loop has formed in the window
+     hierarchy.  */
+
+  parent = child->transient_for;
+  for (; parent; parent = parent->transient_for)
+    {
+      /* If parent becomes child again, then we know there is a
+	 circular reference somewhere.  In that case, post a fatal
+	 protocol error.  */
+
+      if (child == parent)
+	wl_resource_post_error (resource, XDG_TOPLEVEL_ERROR_INVALID_PARENT,
+				"trying to set parent in a circular fashion");
+    }
 }
 
 static void