Merge lp:~3v1n0/unity/quicklist-keynav-fixes into lp:unity

[Marco Trevisan (Treviño)]

QuicklistView: fixed the quicklist key navigation, to make it consistent with Gtk menus navigation.

The prelight of the items is now completely controlled by QuicklistView and never by QuicklistMenuItem, the children can define now when they are slectable.

Quicklist key navigation was broken in multiple aspect, fixed the attached bugs to make it consistent with Gtk menu navigation.

The prelight of the items is now completely controlled by QuicklistView and never by QuicklistMenuItem, the children can define now when they are slectable.

Included Autopilot tests for the QL key navigation, and updated unit-tests for QuicklistMenuItem.

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

82 +bool
83 +QuicklistMenuItem::GetSelectable()

199 +bool
200 +QuicklistMenuItemSeparator::GetSelectable()

238 +void
239 +QuicklistView::SelectItem(int index)

263 bool
264 -QuicklistView::IsMenuItemSeperator(int index)
265 +QuicklistView::IsMenuItemSelectable(int index)

Coding style says these should be on the same name.

667 + def get_items(self, visible_only=True):
668 + """Returns the quicklist items"""
669 + if visible_only:
670 + return self.get_children_by_type(QuicklistMenuItem, visible=True)
671 + else:
672 + return self.get_children_by_type(QuicklistMenuItem)
673 +
674 @property
675 def items(self):
676 """Individual items in the quicklist."""
677 - return self.get_children_by_type(QuicklistMenuItem)
678 + return self.get_items()
679 +
680 + @property
681 + def selectable_items(self):
682 + """Items that can be selected in the quicklist"""
683 + return filter(lambda it: it.selectable == True, self.items)

This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one a method, one a property. Please make them both the same (either methods or properties, I don't mind which). Also, please don't use 'filter' in autopilot. In most places, list comprehensions are clearer. However, in this case, you don't need either - just pass more keyword filters to get_children_by_type (like this):

@property
def selectable_items(self):
    return self.get_children_by_type(QuicklistMenuItem, visible=True, selectable=True)

704 + Mouse().move(target_x, target_y, animate=False)

Unless you have good reason to, please don't pass 'animate=false'
 - this is only intended to be used in exceptional circumstances.

856 + self.assertThat(self.quicklist, Not(Is(None)))
857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))

Please avoid negative assertions. Instead of saying "make sure foo is not None", say "Make sure foo is XYZ" - it makes your tests more specific and robust.

A few notes on docstrings:
 1. All your docstrings need a full-stop (period) at the end.
 2. Please try and make your docstrings PEP257 compliant: http://www.python.org/dev/peps/pep-0257/
 3. You can rewrite many of your docstrings to be shorter, and more forceful. For example, instead of writing this:

"""Tests that the quicklist Home key selects the first valid item when
no item is selected
"""

write this:

"""Home key MUST select the first selectable item in a quicklist."""

Note that this uses the word "must" which is better than "should", "might", or "will", and follows more closely the code of the tests themselves.

Other than that, these look good.

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

> Hi,
>
> 82 +bool
> 83 +QuicklistMenuItem::GetSelectable()
>
> Coding style says these should be on the same name.

Yes I know and I generally do that, but not when I would be inconsistent with the style currently used on the class I'm editing, so I'd prefer not to change it here not to include unneeded changes.
This is something that should be fixed globally on a different coding-style-fixes branch (for Q).

> 667 + def get_items(self, visible_only=True):
> 668 + """Returns the quicklist items"""
> 669 + if visible_only:
> 670 + return self.get_children_by_type(QuicklistMenuItem, visible=True)
> 671 + else:
> 672 + return self.get_children_by_type(QuicklistMenuItem)
> 673 +
> 674 @property
> 675 def items(self):
> 676 """Individual items in the quicklist."""
> 677 - return self.get_children_by_type(QuicklistMenuItem)
> 678 + return self.get_items()
> 679 +
> 680 + @property
> 681 + def selectable_items(self):
> 682 + """Items that can be selected in the quicklist"""
> 683 + return filter(lambda it: it.selectable == True, self.items)
>
>
> This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one
> a method, one a property. Please make them both the same (either methods or
> properties, I don't mind which).

Well, I wanted to keep both for a reason: the get_items() also allow you to get the invisible items of a quicklist, while the property will give you only the "real" result.

> 704 + Mouse().move(target_x, target_y, animate=False)
>
>
> Unless you have good reason to, please don't pass 'animate=false'
> - this is only intended to be used in exceptional circumstances.

I used it to avoid that a quicklist item would have been selected when performing that, adding a possibility to get an invalid result.

> 856 + self.assertThat(self.quicklist, Not(Is(None)))
> 857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))
>
> Please avoid negative assertions. Instead of saying "make sure foo is not
> None", say "Make sure foo is XYZ" - it makes your tests more specific and
> robust.

I wouldn't have used, but it's just a copy and paste from your code of the same class that I used not to be wrong :D

Thanks for the review, I'll update the branch soon.

[Thomi Richards (thomir-deactivatedaccount)]

> > Hi,
> >
> > 82 +bool
> > 83 +QuicklistMenuItem::GetSelectable()
> >
> > Coding style says these should be on the same name.
>
> Yes I know and I generally do that, but not when I would be inconsistent with
> the style currently used on the class I'm editing, so I'd prefer not to change
> it here not to include unneeded changes.
> This is something that should be fixed globally on a different coding-style-
> fixes branch (for Q).

OK, fine with me.
> >
> > This is a bit inconsistent - you have 'get_items' and 'selectable_items' -
> one
> > a method, one a property. Please make them both the same (either methods or
> > properties, I don't mind which).
>
> Well, I wanted to keep both for a reason: the get_items() also allow you to
> get the invisible items of a quicklist, while the property will give you only
> the "real" result.
>

You can still have both - just either make them both properties, or both methods. Probably making them both properties makes more sense.

> > 704 + Mouse().move(target_x, target_y, animate=False)
> >
> >
> > Unless you have good reason to, please don't pass 'animate=false'
> > - this is only intended to be used in exceptional circumstances.
>
> I used it to avoid that a quicklist item would have been selected when
> performing that, adding a possibility to get an invalid result.
>

Fair enough.

> > 856 + self.assertThat(self.quicklist, Not(Is(None)))
> > 857 + self.assertThat(self.quicklist.selected_item, Not(Is(None)))
> >
> > Please avoid negative assertions. Instead of saying "make sure foo is not
> > None", say "Make sure foo is XYZ" - it makes your tests more specific and
> > robust.
>
> I wouldn't have used, but it's just a copy and paste from your code of the
> same class that I used not to be wrong :D
>

Yeah, my code's not perfect either. Negative assertions are something that I've come to realise are really bad very recently, so older code is likely to be wrong.

Cheers,

[Marco Trevisan (Treviño) (3v1n0)]

> This is a bit inconsistent - you have 'get_items' and 'selectable_items' - one
> a method, one a property. Please make them both the same (either methods or
> properties, I don't mind which).

Well, I did that because I also wanted to include a method to fetch the invisible items, but I'll remove it since no one uses it yet.

> Also, please don't use 'filter' in autopilot.

Yes you're right, I used it only because at the begginning selectable was a property that I defined only on the emulator, then I moved it to the introspection too, and I forgot to update the code.

I've updated the branch as well, I hope it's fine now ;)

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

There are still some issues with this I'm afraid:

672 + """Items that can be selected in the quicklist"""

Needs a full-stop at the end of the docstring.

727 + def mouse_click(self, button=1):
728 + logger.debug("Clicking on quicklist item %r", self)
729 + self.mouse_move_to()
730 + sleep(.25)
731 + assert(self.visible)
732 + self._mouse.click(press_duration=.1)
733 + sleep(.25)

Several issues here:
 1) The assert should be at the start of this method, not half way through.
 2) You don't need the sleep() statements, please remove them.
 3) You don't need to specify the click duration. Please just use "mouse.click()".

66 + for icon in self.launcher.model.get_launcher_icons():
867 + if (icon.tooltip_text != self.ql_app.name):
868 + self.ql_launcher.key_nav_next()
869 + else:
870 + self.keybinding("launcher/keynav/open-quicklist")
871 + self.addCleanup(self.keybinding, "launcher/keynav/close-quicklist")
872 + break

Instead of iterating over the model icons, please use the method on the launcher model, like this:

icon = self.launcher.model.get_icon_by_desktop_id(...)

If you use the desktop Id then we won't have issues in didfferent locales.

885 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
899 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
908 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
917 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
931 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
940 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
949 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
958 + self.assertTrue(item.selected)
959 + self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
979 + self.assertTrue(mouse_item.selected)
984 + self.assertTrue(expected_item.selected)
985 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
995 + self.assertTrue(mouse_item.selected)
1000 + self.assertTrue(expected_item.selected)
1001 + self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
1010 + self.assertTrue(mouse_item.selected)
1017 + self.assertTrue(key_item.selected)
1018 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1023 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1027 + self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
1032 + self.assertTrue(mouse_item.selected)
1033 + self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))

Please use the new Eventually() matcher for all of these. To import it:

from autopilot.matchers import Eventually

to use it, take this:

self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))

and turn it into this:

self.assertThat(self.quicklist.selected_item.id, Eventually(Equals(mouse_item.id)))

Eventually takes any mater, so you can also do things like this:

self.assertThat(self.quicklist.selected_item.id, Eventually(LessThan(mouse_item.id)))

Finally, please remove the sleep() statements. Using the Eventually(...) matcher removes the n...

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

> Needs a full-stop at the end of the docstring.

Ouch!

> 727 + def mouse_click(self, button=1):
> Several issues here:
> 1) The assert should be at the start of this method, not half way through.
> 2) You don't need the sleep() statements, please remove them.
> 3) You don't need to specify the click duration. Please just use
> "mouse.click()".

Done

> 66 + for icon in self.launcher.model.get_launcher_icons():

> Instead of iterating over the model icons, please use the method on the
> launcher model, like this:
>
> icon = self.launcher.model.get_icon_by_desktop_id(...)

I can't... That was a keyboard selection... I must do that.

> If you use the desktop Id then we won't have issues in didfferent locales.

Using the bamf name we don't have these issue... Tested here, with Italian locale ;)

> 885 + self.assertThat(self.quicklist.selected_item.id,
> Equals(expected_item.id))

> Please use the new Eventually() matcher for all of these. To import it:
> Finally, please remove the sleep() statements. Using the Eventually(...)
> matcher removes the need for random sleep statements in your tests.

Done ;)

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

[Unity Merger (unity-merger)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'plugins/unityshell/src/AbstractLauncherIcon.h'
--- plugins/unityshell/src/AbstractLauncherIcon.h	2012-03-30 15:24:34 +0000
+++ plugins/unityshell/src/AbstractLauncherIcon.h	2012-04-19 00:02:18 +0000
@@ -128,7 +128,7 @@
 
   virtual void SetSortPriority(int priority) = 0;
 
-  virtual bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1) = 0;
+  virtual bool OpenQuicklist(bool select_first_item = false, int monitor = -1) = 0;
 
   virtual void        SetCenter(nux::Point3 center, int monitor, nux::Geometry parent_geo) = 0;
 
 
=== modified file 'plugins/unityshell/src/LauncherIcon.cpp'
--- plugins/unityshell/src/LauncherIcon.cpp	2012-04-10 00:39:14 +0000
+++ plugins/unityshell/src/LauncherIcon.cpp	2012-04-19 00:02:18 +0000
@@ -537,7 +537,7 @@
     _tooltip->ShowWindow(false);
 }
 
-bool LauncherIcon::OpenQuicklist(bool default_to_first_item, int monitor)
+bool LauncherIcon::OpenQuicklist(bool select_first_item, int monitor)
 {
   std::list<DbusmenuMenuitem*> menus = Menus();
 
@@ -583,8 +583,8 @@
     _quicklist->AddMenuItem(ql_item);
   }
 
-  if (default_to_first_item)
-    _quicklist->DefaultToFirstItem();
+  if (select_first_item)
+    _quicklist->SelectFirstItem();
 
   if (monitor < 0)
   {
 
=== modified file 'plugins/unityshell/src/LauncherIcon.h'
--- plugins/unityshell/src/LauncherIcon.h	2012-04-03 22:18:36 +0000
+++ plugins/unityshell/src/LauncherIcon.h	2012-04-19 00:02:18 +0000
@@ -86,7 +86,7 @@
 
   void ShowTooltip();
 
-  bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1);
+  bool OpenQuicklist(bool select_first_item = false, int monitor = -1);
 
   void        SetCenter(nux::Point3 center, int parent_monitor, nux::Geometry parent_geo);
 
 
=== modified file 'plugins/unityshell/src/MockLauncherIcon.h'
--- plugins/unityshell/src/MockLauncherIcon.h	2012-03-30 15:24:34 +0000
+++ plugins/unityshell/src/MockLauncherIcon.h	2012-04-19 00:02:18 +0000
@@ -120,7 +120,7 @@
 
   void SetSortPriority(int priority) { sort_priority_ = priority; }
 
-  bool OpenQuicklist(bool default_to_first_item = false, int monitor = -1)
+  bool OpenQuicklist(bool select_first_item = false, int monitor = -1)
   {
     return false;
   }
 
=== modified file 'plugins/unityshell/src/QuicklistMenuItem.cpp'
--- plugins/unityshell/src/QuicklistMenuItem.cpp	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistMenuItem.cpp	2012-04-19 00:02:18 +0000
@@ -89,10 +89,8 @@
                      this);
   }
 
-  mouse_down.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseDown));
   mouse_up.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseUp));
   mouse_click.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseClick));
-  mouse_move.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseMove));
   mouse_drag.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseDrag));
   mouse_enter.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseEnter));
   mouse_leave.connect(sigc::mem_fun(this, &QuicklistMenuItem::RecvMouseLeave));
@@ -215,6 +213,12 @@
                                              DBUSMENU_MENUITEM_PROP_VISIBLE);
 }
 
+bool
+QuicklistMenuItem::GetSelectable()
+{
+  return GetVisible() && GetEnabled();
+}
+
 void QuicklistMenuItem::ItemActivated()
 {
   if (_debug)
@@ -333,11 +337,6 @@
   //self->ItemActivated ();
 }
 
-void QuicklistMenuItem::RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags)
-{
-
-}
-
 void QuicklistMenuItem::RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags)
 {
   sigMouseReleased.emit(this, x, y);
@@ -352,11 +351,6 @@
   sigMouseClick.emit(this, x, y);
 }
 
-void QuicklistMenuItem::RecvMouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags)
-{
-
-}
-
 void QuicklistMenuItem::RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags)
 {
   sigMouseDrag.emit(this, x, y);
@@ -364,13 +358,11 @@
 
 void QuicklistMenuItem::RecvMouseEnter(int x, int y, unsigned long button_flags, unsigned long key_flags)
 {
-  _prelight = true;
   sigMouseEnter.emit(this);
 }
 
 void QuicklistMenuItem::RecvMouseLeave(int x, int y, unsigned long button_flags, unsigned long key_flags)
 {
-  _prelight = false;
   sigMouseLeave.emit(this);
 }
 
@@ -484,20 +476,14 @@
 
 void QuicklistMenuItem::AddProperties(GVariantBuilder* builder)
 {
-  nux::Geometry abs_geo = GetAbsoluteGeometry();
-  
   unity::variant::BuilderWrapper(builder)
-  .add("absolute_x", abs_geo.x)
-  .add("absolute_y", abs_geo.y)
+  .add(GetAbsoluteGeometry())
   .add("text", _text)
-  .add("x", GetBaseX())
-  .add("y", GetBaseY())
-  .add("width", GetBaseWidth())
-  .add("height", GetBaseHeight())
   .add("enabled", GetEnabled())
   .add("active", GetActive())
   .add("visible", GetVisible())
-  .add("lit", _prelight);
+  .add("selectable", GetSelectable())
+  .add("selected", _prelight);
 }
 
 } //NAMESPACE
 
=== modified file 'plugins/unityshell/src/QuicklistMenuItem.h'
--- plugins/unityshell/src/QuicklistMenuItem.h	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistMenuItem.h	2012-04-19 00:02:18 +0000
@@ -85,11 +85,13 @@
   virtual bool GetEnabled();
   virtual bool GetActive();
   virtual bool GetVisible();
+  virtual bool GetSelectable();
 
+protected:
   // Introspection
   std::string GetName() const;
   void AddProperties(GVariantBuilder* builder);
-protected:
+
   static const int ITEM_INDENT_ABS = 16;
   static const int ITEM_CORNER_RADIUS_ABS = 3;
   static const int ITEM_MARGIN = 4;
@@ -116,10 +118,8 @@
 
   void RecvMouseEnter(int x, int y, unsigned long button_flags, unsigned long key_flags);
   void RecvMouseLeave(int x, int y, unsigned long button_flags, unsigned long key_flags);
-  void RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags);
   void RecvMouseUp(int x, int y, unsigned long button_flags, unsigned long key_flags);
   void RecvMouseClick(int x, int y, unsigned long button_flags, unsigned long key_flags);
-  void RecvMouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
   void RecvMouseDrag(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags);
 
   sigc::signal<void, QuicklistMenuItem*> sigMouseEnter;
@@ -134,7 +134,6 @@
   nux::Color        _color;   //!< Item rendering color factor.
   bool              _debug;
 
-
   bool _prelight;   //!< True when the mouse is over the item.
 
   void DrawText(nux::CairoGraphics* cairo, int width, int height, nux::Color const& color);
 
=== modified file 'plugins/unityshell/src/QuicklistMenuItemSeparator.cpp'
--- plugins/unityshell/src/QuicklistMenuItemSeparator.cpp	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistMenuItemSeparator.cpp	2012-04-19 00:02:18 +0000
@@ -59,6 +59,12 @@
 {
 }
 
+bool
+QuicklistMenuItemSeparator::GetSelectable()
+{
+  return false;
+}
+
 void
 QuicklistMenuItemSeparator::PreLayoutManagement()
 {
 
=== modified file 'plugins/unityshell/src/QuicklistMenuItemSeparator.h'
--- plugins/unityshell/src/QuicklistMenuItemSeparator.h	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistMenuItemSeparator.h	2012-04-19 00:02:18 +0000
@@ -43,6 +43,8 @@
 
   ~QuicklistMenuItemSeparator();
 
+  virtual bool GetSelectable();
+
 protected:
 
   void PreLayoutManagement();
 
=== modified file 'plugins/unityshell/src/QuicklistView.cpp'
--- plugins/unityshell/src/QuicklistView.cpp	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistView.cpp	2012-04-19 00:02:18 +0000
@@ -69,7 +69,7 @@
   , _bottom_padding_correction_single_item(-4)
   , _offset_correction(-1)
   , _cairo_text_has_changed(true)
-  , _current_item_index(0)
+  , _current_item_index(-1)
 {
   SetGeometry(nux::Geometry(0, 0, 1, 1));
 
@@ -138,13 +138,35 @@
 {
 }
 
+void
+QuicklistView::SelectItem(int index)
+{
+  CancelItemsPrelightStatus();
+  int target_item = -1;
+
+  if (IsMenuItemSelectable(index))
+  {
+    QuicklistMenuItem* menu_item = GetNthItems(index);
+
+    if (menu_item)
+    {
+      target_item = index;
+      menu_item->_prelight = true;
+    }
+  }
+
+  if (_current_item_index != target_item)
+  {
+    _current_item_index = target_item;
+    selection_change.emit();
+    QueueDraw();
+  }
+}
+
 bool
-QuicklistView::IsMenuItemSeperator(int index)
+QuicklistView::IsMenuItemSelectable(int index)
 {
-  QuicklistMenuItem* menu_item = NULL;
-  DbusmenuMenuitem*  item   = NULL;
-  const gchar*       label  = NULL;
-  bool               result = false;
+  QuicklistMenuItem* menu_item = nullptr;
 
   if (index < 0)
     return false;
@@ -153,17 +175,7 @@
   if (!menu_item)
     return false;
 
-  item = menu_item->_menuItem;
-  if (!item)
-    return false;
-
-  label = dbusmenu_menuitem_property_get(item, DBUSMENU_MENUITEM_PROP_LABEL);
-  if (!label)
-    result = true;
-  else
-    result = false;
-
-  return result;
+  return menu_item->GetSelectable();
 }
 
 void
@@ -175,56 +187,103 @@
 {
   switch (key_sym)
   {
+      // home or page up (highlight the first menu-hitem)
+    case NUX_VK_PAGE_UP:
+    case NUX_VK_HOME:
+    {
+      int num_items = GetNumItems();
+      int target_index = -1;
+
+      do
+      {
+        ++target_index;
+      }
+      while (!IsMenuItemSelectable(target_index) && target_index < num_items);
+
+      if (target_index < num_items)
+        SelectItem(target_index);
+
+      break;
+    }
+      // end or page down (highlight the last menu-hitem)
+    case NUX_VK_PAGE_DOWN:
+    case NUX_VK_END:
+    {
+      int target_index = GetNumItems();
+
+      do
+      {
+        --target_index;
+      }
+      while (!IsMenuItemSelectable(target_index) && target_index >= 0);
+
+      if (target_index >= 0)
+        SelectItem(target_index);
+
+      break;
+    }
       // up (highlight previous menu-item)
     case NUX_VK_UP:
     case NUX_KP_UP:
-      // protect against edge-case of first item being a separator
-      if (_current_item_index == 1 && IsMenuItemSeperator(0))
-        break;
-
-      if (_current_item_index > 0)
+    {
+      int target_index = _current_item_index;
+      bool loop_back = false;
+
+      if (target_index <= 0)
+        target_index = GetNumItems();
+
+      do
       {
-        GetNthItems(_current_item_index)->_prelight = false;
-        _current_item_index--;
-
-        while (IsMenuItemSeperator(_current_item_index))
-          _current_item_index--;
-
-        selection_change.emit();
-
-        GetNthItems(_current_item_index)->_prelight = true;
-        QueueDraw();
+        --target_index;
+
+        // If the first item is not selectable, we must loop from the last one
+        if (!loop_back && target_index == 0 && !IsMenuItemSelectable(target_index))
+        {
+          loop_back = true;
+          target_index = GetNumItems() - 1;
+        }
       }
+      while (!IsMenuItemSelectable(target_index) && target_index >= 0);
+
+      if (target_index >= 0)
+        SelectItem(target_index);
+
       break;
+    }
 
       // down (highlight next menu-item)
     case NUX_VK_DOWN:
     case NUX_KP_DOWN:
-      // protect against edge-case of last item being a separator
-      if (_current_item_index == (GetNumItems() - 1) && IsMenuItemSeperator(GetNumItems()))
-        break;
-
-      if (_current_item_index < GetNumItems() - 1)
+    {
+      int target_index = _current_item_index;
+      int num_items = GetNumItems();
+      bool loop_back = false;
+
+      if (target_index >= num_items - 1)
+        target_index = -1;
+
+      do
       {
-        GetNthItems(_current_item_index)->_prelight = false;
-        _current_item_index++;
-
-        while (IsMenuItemSeperator(_current_item_index))
-          _current_item_index++;
-
-        selection_change.emit();
-
-        GetNthItems(_current_item_index)->_prelight = true;
-        QueueDraw();
+        ++target_index;
+
+        // If the last item is not selectable, we must loop from the first one
+        if (!loop_back && target_index == num_items - 1 && !IsMenuItemSelectable(target_index))
+        {
+          loop_back = true;
+          target_index = 0;
+        }
       }
+      while (!IsMenuItemSelectable(target_index) && target_index < num_items);
+
+      if (target_index < num_items)
+        SelectItem(target_index);
+
       break;
+    }
 
       // left (close quicklist, go back to laucher key-nav)
     case NUX_VK_LEFT:
     case NUX_KP_LEFT:
-      _current_item_index = 0;
-      selection_change.emit();
-      GetNthItems(_current_item_index)->_prelight = true;
       Hide();
       // inform Launcher we switch back to Launcher key-nav
       ubus_server_send_message(ubus_server_get_default(),
@@ -234,32 +293,23 @@
 
       // esc (close quicklist, exit key-nav)
     case NUX_VK_ESCAPE:
-      _current_item_index = 0;
-      selection_change.emit();
-      GetNthItems(_current_item_index)->_prelight = true;
       Hide();
       // inform UnityScreen we leave key-nav completely
       ubus_server_send_message(ubus_server_get_default(),
                                UBUS_LAUNCHER_END_KEY_NAV,
                                NULL);
-      selection_change.emit();
       break;
 
       // <SPACE>, <RETURN> (activate selected menu-item)
     case NUX_VK_SPACE:
     case NUX_VK_ENTER:
     case NUX_KP_ENTER:
-      if (_current_item_index >= 0 && _current_item_index < GetNumItems() &&
-          GetNthItems(_current_item_index)->GetEnabled())
+      if (IsMenuItemSelectable(_current_item_index))
       {
-
         dbusmenu_menuitem_handle_event(GetNthItems(_current_item_index)->_menuItem,
                                        "clicked",
                                        NULL,
                                        0);
-        _current_item_index = 0;
-        selection_change.emit();
-        GetNthItems(_current_item_index)->_prelight = true;
         Hide();
       }
       break;
@@ -365,6 +415,12 @@
     UnGrabKeyboard();
     //EnableInputWindow (false);
     ShowWindow(false);
+
+    if (_current_item_index != -1)
+    {
+      selection_change.emit();
+      _current_item_index = -1;
+    }
   }
 }
 
@@ -618,54 +674,53 @@
 void QuicklistView::RecvItemMouseDrag(QuicklistMenuItem* item, int x, int y)
 {
   nux::Geometry geo;
-  std::list<QuicklistMenuItem*>::iterator it;
-  for (it = _item_list.begin(); it != _item_list.end(); it++)
+
+  for (auto it : _item_list)
   {
-    if (!(*it)->GetVisible())
+    int item_index = GetItemIndex(it);
+
+    if (!IsMenuItemSelectable(item_index))
       continue;
 
-    geo = (*it)->GetGeometry();
+    geo = it->GetGeometry();
     geo.width = _item_layout->GetBaseWidth();
 
     if (geo.IsPointInside(x + item->GetBaseX(), y + item->GetBaseY()))
     {
-      (*it)->_prelight = true;
-    }
-    else
-    {
-      (*it)->_prelight = false;
+      SelectItem(item_index);
     }
   }
 
-  for (it = _default_item_list.begin(); it != _default_item_list.end(); it++)
+  for (auto it : _default_item_list)
   {
-    if (!(*it)->GetVisible())
+    int item_index = GetItemIndex(it);
+
+    if (!IsMenuItemSelectable(item_index))
       continue;
 
-    geo = (*it)->GetGeometry();
+    geo = it->GetGeometry();
     geo.width = _default_item_layout->GetBaseWidth();
 
     if (geo.IsPointInside(x + item->GetBaseX(), y + item->GetBaseY()))
     {
-      (*it)->_prelight = true;
-    }
-    else
-    {
-      (*it)->_prelight = false;
+      SelectItem(item_index);
     }
   }
-
-  NeedRedraw();
 }
 
 void QuicklistView::RecvItemMouseEnter(QuicklistMenuItem* item)
 {
-  NeedRedraw();
+  int item_index = GetItemIndex(item);
+
+  SelectItem(item_index);
 }
 
 void QuicklistView::RecvItemMouseLeave(QuicklistMenuItem* item)
 {
-  NeedRedraw();
+  int item_index = GetItemIndex(item);
+
+  if (item_index < 0 || item_index == _current_item_index)
+    SelectItem(-1);
 }
 
 void QuicklistView::RecvMouseDown(int x, int y, unsigned long button_flags, unsigned long key_flags)
@@ -786,14 +841,37 @@
     if (_item_list.size() > 0)
       i = _item_list.size() - 1;
     std::list<QuicklistMenuItem*>::iterator it;
-    for (it = _item_list.begin(); it != _item_list.end(); i++, it++)
+    for (it = _default_item_list.begin(); it != _default_item_list.end(); i++, it++)
     {
       if (i == index)
         return *it;
     }
   }
 
-  return 0;
+  return nullptr;
+}
+
+int QuicklistView::GetItemIndex(QuicklistMenuItem* item)
+{
+  int index = -1;
+
+  for (auto it : _item_list)
+  {
+    ++index;
+
+    if (it == item)
+      return index;
+  }
+
+  for (auto it : _default_item_list)
+  {
+    ++index;
+
+    if (it == item)
+      return index;
+  }
+
+  return index;
 }
 
 QuicklistMenuItemType QuicklistView::GetNthType(int index)
@@ -801,6 +879,7 @@
   QuicklistMenuItem* item = GetNthItems(index);
   if (item)
     return item->GetItemType();
+
   return MENUITEM_TYPE_UNKNOWN;
 }
 
@@ -809,13 +888,9 @@
   return _item_list;
 }
 
-void QuicklistView::DefaultToFirstItem()
+void QuicklistView::SelectFirstItem()
 {
-  if (GetNumItems() >= 1)
-  {
-    GetNthItems(0)->_prelight = true;
-    QueueDraw();
-  }
+  SelectItem(0);
 }
 
 void ql_tint_dot_hl(cairo_t* cr,
@@ -1400,15 +1475,10 @@
 
 void QuicklistView::AddProperties(GVariantBuilder* builder)
 {
-  nux::Geometry abs_geo = GetAbsoluteGeometry();
-
   variant::BuilderWrapper(builder)
-    .add("x", abs_geo.x)
-    .add("y", abs_geo.y)
+    .add(GetAbsoluteGeometry())
     .add("base_x", GetBaseX())
     .add("base_y", GetBaseY())
-    .add("width", GetBaseWidth())
-    .add("height", GetBaseHeight())
     .add("active", IsVisible());
 }
 
 
=== modified file 'plugins/unityshell/src/QuicklistView.h'
--- plugins/unityshell/src/QuicklistView.h	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/QuicklistView.h	2012-04-19 00:02:18 +0000
@@ -62,8 +62,9 @@
   int GetNumItems();
   QuicklistMenuItem* GetNthItems(int index);
   QuicklistMenuItemType GetNthType(int index);
+  int GetItemIndex(QuicklistMenuItem* item);
   std::list<QuicklistMenuItem*> GetChildren();
-  void DefaultToFirstItem();
+  void SelectFirstItem();
 
   void TestMenuItems(DbusmenuMenuitem* root);
 
@@ -130,7 +131,8 @@
   //! Check the mouse up event sent by an item. Detect the item where the mous is and emit the appropriate signal.
   void CheckAndEmitItemSignal(int x, int y);
 
-  bool IsMenuItemSeperator(int index);
+  void SelectItem(int index);
+  bool IsMenuItemSelectable(int index);
 
   //nux::CairoGraphics*   _cairo_graphics;
   int                   _anchorX;
 
=== modified file 'tests/autopilot/autopilot/emulators/unity/quicklist.py'
--- tests/autopilot/autopilot/emulators/unity/quicklist.py	2012-03-06 21:26:11 +0000
+++ tests/autopilot/autopilot/emulators/unity/quicklist.py	2012-04-19 00:02:18 +0000
@@ -23,7 +23,12 @@
     @property
     def items(self):
         """Individual items in the quicklist."""
-        return self.get_children_by_type(QuicklistMenuItem)
+        return self.get_children_by_type(QuicklistMenuItem, visible=True)
+
+    @property
+    def selectable_items(self):
+        """Items that can be selected in the quicklist."""
+        return self.get_children_by_type(QuicklistMenuItem, visible=True, selectable=True)
 
     def get_quicklist_item_by_text(self, text):
         """Returns a QuicklistMenuItemLabel object with the given text, or None."""
@@ -40,17 +45,51 @@
         if not isinstance(item, QuicklistMenuItem):
             raise TypeError("Item must be a subclass of QuicklistMenuItem")
 
-        logger.debug("Clicking on quicklist item %r", item)
-        mouse = Mouse()
-        mouse.move(self.x + item.x + (item.width / 2),
-                    self.y + item.y + (item.height / 2))
-        sleep(0.25)
-        mouse.click()
+        item.mouse_click()
+
+    def move_mouse_to_right(self):
+        """Moves the mouse outside the quicklist"""
+        logger.debug("Moving mouse outside the quicklist %r", self)
+        target_x = self.x + self.width + 10
+        target_y = self.y + self.height / 2
+        Mouse().move(target_x, target_y, animate=False)
+
+    @property
+    def selected_item(self):
+        items = self.get_children_by_type(QuicklistMenuItem, selected=True)
+        assert(len(items) <= 1)
+        return items[0] if items else None
+
+    @property
+    def geometry(self):
+        """Returns a tuple of (x,y,w,h) for the quicklist."""
+        return (self.x, self.y, self.width, self.height)
 
 
 class QuicklistMenuItem(UnityIntrospectionObject):
     """Represents a single item in a quicklist."""
 
+    def __init__(self, *args, **kwargs):
+        super(QuicklistMenuItem, self).__init__(*args, **kwargs)
+        self._mouse = Mouse()
+
+    @property
+    def geometry(self):
+        """Returns a tuple of (x,y,w,h) for the quicklist item."""
+        return (self.x, self.y, self.width, self.height)
+
+    def mouse_move_to(self):
+        assert(self.visible)
+        logger.debug("Moving mouse over quicklist item %r", self)
+        target_x = self.x + self.width / 2
+        target_y = self.y + self.height / 2
+        self._mouse.move(target_x, target_y, rate=20, time_between_events=0.005)
+
+    def mouse_click(self, button=1):
+        logger.debug("Clicking on quicklist item %r", self)
+        self.mouse_move_to()
+        self._mouse.click()
+
 
 class QuicklistMenuItemLabel(QuicklistMenuItem):
     """Represents a text label inside a quicklist."""
 
=== modified file 'tests/autopilot/autopilot/keybindings.py'
--- tests/autopilot/autopilot/keybindings.py	2012-04-06 01:28:24 +0000
+++ tests/autopilot/autopilot/keybindings.py	2012-04-19 00:02:18 +0000
@@ -56,7 +56,14 @@
     "launcher/switcher/prev": "Shift+Tab",
     "launcher/switcher/down": "Down",
     "launcher/switcher/up": "Up",
-    # Panel
+    # Quicklist:
+    "quicklist/keynav/first": "Home",
+    "quicklist/keynav/last": "End",
+    "quicklist/keynav/next": "Down",
+    "quicklist/keynav/prev": "Up",
+    "quicklist/keynav/activate": "Enter",
+    "quicklist/keynav/exit": "Escape",
+    # Panel:
     "panel/show_menus": "Alt",
     "panel/open_first_menu": ('unityshell', 'panel_first_menu'),
     "panel/next_indicator": "Right",
@@ -65,12 +72,12 @@
     "dash/reveal": "Super",
     "dash/lens/next": "Ctrl+Tab",
     "dash/lens/prev": "Ctrl+Shift+Tab",
-    # Lenses
+    # Lenses:
     "lens_reveal/command": ("unityshell", "execute_command"),
     "lens_reveal/apps": "Super+a",
     "lens_reveal/files": "Super+f",
     "lens_reveal/music": "Super+m",
-    # Hud
+    # Hud:
     "hud/reveal": ("unityshell", "show_hud"),
     # Switcher:
     "switcher/reveal_normal": ("unityshell", "alt_tab_forward"),
@@ -96,7 +103,7 @@
     "workspace/move_right": ("wall", "right_key"),
     "workspace/move_up": ("wall", "up_key"),
     "workspace/move_down": ("wall", "down_key"),
-    # Window management
+    # Window management:
     "window/show_desktop" : ("core", "show_desktop_key"),
     "window/minimize": ("core", "minimize_window_key"),
     "window/maximize": ("core", "maximize_window_key"),
 
=== modified file 'tests/autopilot/autopilot/tests/test_quicklist.py'
--- tests/autopilot/autopilot/tests/test_quicklist.py	2012-04-02 20:18:58 +0000
+++ tests/autopilot/autopilot/tests/test_quicklist.py	2012-04-19 00:02:18 +0000
@@ -1,16 +1,19 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 # Copyright 2012 Canonical
-# Author: Thomi Richards
+# Author: Thomi Richards,
+#         Marco Trevisan (Treviño)
 #
 # 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.
 
 import os.path
-from testtools.matchers import Not, Is, Contains
+from testtools.matchers import Contains, Equals, NotEquals
 from xdg.DesktopEntry import DesktopEntry
+from time import sleep
 
 from autopilot.emulators.unity.quicklist import QuicklistMenuItemLabel
+from autopilot.matchers import Eventually
 from autopilot.tests import AutopilotTestCase
 
 
@@ -26,22 +29,23 @@
         self.start_app(self.app_name)
 
         # load the desktop file from disk:
-        desktop_file = os.path.join('/usr/share/applications',
-            self.KNOWN_APPS[self.app_name]['desktop-file']
-            )
+        desktop_id = self.KNOWN_APPS[self.app_name]['desktop-file']
+        desktop_file = os.path.join('/usr/share/applications', desktop_id)
         de = DesktopEntry(desktop_file)
         # get the launcher icon from the launcher:
-        launcher_icon = self.launcher.model.get_icon_by_tooltip_text(de.getName())
-        self.assertThat(launcher_icon, Not(Is(None)))
+        launcher_icon = self.launcher.model.get_icon_by_desktop_id(desktop_id)
+        self.assertThat(launcher_icon, NotEquals(None))
 
         # open the icon quicklist, and get all the text labels:
         launcher = self.launcher.get_launcher_for_monitor(0)
         launcher.click_launcher_icon(launcher_icon, button=3)
+        self.addCleanup(self.keyboard.press_and_release, "Escape")
         ql = launcher_icon.get_quicklist()
         ql_item_texts = [i.text for i in ql.items if type(i) is QuicklistMenuItemLabel]
 
         # iterate over all the actions from the desktop file, make sure they're
         # present in the quicklist texts.
+        # FIXME, this doesn't work using a locale other than English.
         actions = de.getActions()
         for action in actions:
             key = 'Desktop Action ' + action
@@ -50,3 +54,197 @@
             self.assertThat(ql_item_texts, Contains(name))
 
 
+class QuicklistKeyNavigationTests(AutopilotTestCase):
+    """Tests for the quicklist key navigation."""
+
+    def setUp(self):
+        super(QuicklistKeyNavigationTests, self).setUp()
+
+        self.ql_app = self.start_app("Text Editor")
+
+        self.ql_launcher_icon = self.launcher.model.get_icon_by_desktop_id(self.ql_app.desktop_file)
+        self.assertThat(self.ql_launcher_icon, NotEquals(None))
+
+        self.ql_launcher = self.launcher.get_launcher_for_monitor(0)
+
+    def open_quicklist_with_mouse(self):
+        """Opens a quicklist with the mouse."""
+        self.ql_launcher.click_launcher_icon(self.ql_launcher_icon, button=3)
+        self.addCleanup(self.keyboard.press_and_release, "Escape")
+        self.quicklist = self.ql_launcher_icon.get_quicklist()
+        self.assertThat(self.quicklist, NotEquals(None))
+        self.quicklist.move_mouse_to_right()
+        self.assertThat(self.quicklist.selected_item, Equals(None))
+
+    def open_quicklist_with_keyboard(self):
+        """Opens a quicklist using the keyboard."""
+        self.screen_geo.move_mouse_to_monitor(0)
+        self.ql_launcher.key_nav_start()
+        self.addCleanup(self.ql_launcher.key_nav_cancel)
+
+        for icon in self.launcher.model.get_launcher_icons():
+            if icon.tooltip_text != self.ql_app.name:
+                self.ql_launcher.key_nav_next()
+            else:
+                self.keybinding("launcher/keynav/open-quicklist")
+                self.addCleanup(self.keybinding, "launcher/keynav/close-quicklist")
+                break
+
+        self.quicklist = self.ql_launcher_icon.get_quicklist()
+        self.assertThat(self.quicklist, NotEquals(None))
+        self.assertThat(self.quicklist.selected_item, NotEquals(None))
+
+    def test_keynav_selects_first_item_when_unselected(self):
+        """Home key MUST select the first selectable item in a quicklist."""
+        self.open_quicklist_with_mouse()
+
+        self.keybinding("quicklist/keynav/first")
+
+        expected_item = self.quicklist.selectable_items[0]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_selects_first_item_when_selected(self):
+        """Home key MUST select the first selectable item in a quicklist when
+        another item is selected.
+        """
+        self.open_quicklist_with_mouse()
+        mouse_item = self.quicklist.selectable_items[-1]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+
+        self.keybinding("quicklist/keynav/first")
+
+        expected_item = self.quicklist.selectable_items[0]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_next_selects_first_item_when_unselected(self):
+        """Down key MUST select the first valid item when nothing is selected."""
+        self.open_quicklist_with_mouse()
+
+        self.keybinding("quicklist/keynav/next")
+
+        expected_item = self.quicklist.selectable_items[0]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_selects_last_item_when_unselected(self):
+        """End key MUST select the last selectable item in a quicklist."""
+        self.open_quicklist_with_mouse()
+
+        self.keybinding("quicklist/keynav/last")
+
+        expected_item = self.quicklist.selectable_items[-1]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_selects_last_item_when_selected(self):
+        """End key MUST select the last selectable item in a quicklist when
+        another item is selected.
+        """
+        self.open_quicklist_with_mouse()
+        mouse_item = self.quicklist.selectable_items[0]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+
+        self.keybinding("quicklist/keynav/last")
+
+        expected_item = self.quicklist.selectable_items[-1]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_prev_selects_last_item_when_unselected(self):
+        """Up key MUST select the last valid item when nothing is selected."""
+        self.open_quicklist_with_mouse()
+
+        self.keybinding("quicklist/keynav/prev")
+
+        expected_item = self.quicklist.selectable_items[-1]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_launcher_keynav_selects_first_item(self):
+        """The first selectable item of the quicklist must be selected when
+        opening the quicklist using the launcher key navigation.
+        """
+        self.open_quicklist_with_keyboard()
+
+        expected_item = self.quicklist.selectable_items[0]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_next_selection_works(self):
+        """Down key MUST select the next valid item."""
+        self.open_quicklist_with_mouse()
+
+        for item in self.quicklist.selectable_items:
+            self.keybinding("quicklist/keynav/next")
+            self.assertThat(item.selected, Eventually(Equals(True)))
+            self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
+
+    def test_keynav_prev_selection_works(self):
+        """Up key MUST select the previous valid item."""
+        self.open_quicklist_with_mouse()
+
+        for item in reversed(self.quicklist.selectable_items):
+            self.keybinding("quicklist/keynav/prev")
+            self.assertThat(item.selected, Eventually(Equals(True)))
+            self.assertThat(self.quicklist.selected_item.id, Equals(item.id))
+
+    def test_keynav_prev_is_cyclic(self):
+        """Up key MUST select the last item, when the first one is selected."""
+        self.open_quicklist_with_mouse()
+
+        mouse_item = self.quicklist.selectable_items[0]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+
+        self.keybinding("quicklist/keynav/prev")
+        expected_item = self.quicklist.selectable_items[-1]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_next_is_cyclic(self):
+        """Down key MUST select the first item, when the last one is selected."""
+        self.open_quicklist_with_mouse()
+
+        mouse_item = self.quicklist.selectable_items[-1]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+
+        self.keybinding("quicklist/keynav/next")
+        expected_item = self.quicklist.selectable_items[0]
+        self.assertThat(expected_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(expected_item.id))
+
+    def test_keynav_mouse_interaction(self):
+        """Tests that the interaction between key-navigation and mouse works as
+        expected. See bug #911561.
+        """
+        self.open_quicklist_with_mouse()
+        mouse_item = self.quicklist.selectable_items[-1]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+
+        self.keybinding("quicklist/keynav/prev")
+        sleep(.1)
+        self.keybinding("quicklist/keynav/prev")
+
+        key_item = self.quicklist.selectable_items[-3]
+        self.assertThat(key_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
+
+        # Moving the mouse horizontally doesn't change the selection
+        self.mouse.move(mouse_item.x + mouse_item.width - 10, mouse_item.y + mouse_item.height / 2)
+        self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
+
+        # Moving the mouse outside doesn't change the selection
+        self.mouse.move(mouse_item.x + mouse_item.width + 50, mouse_item.y + mouse_item.height / 2)
+        self.assertThat(self.quicklist.selected_item.id, Equals(key_item.id))
+
+        # Moving the mouse to another entry, changes the selection
+        mouse_item = self.quicklist.selectable_items[-2]
+        mouse_item.mouse_move_to()
+        self.assertThat(mouse_item.selected, Eventually(Equals(True)))
+        self.assertThat(self.quicklist.selected_item.id, Equals(mouse_item.id))
 
=== modified file 'tests/unit/TestQuicklistMenuitems.cpp'
--- tests/unit/TestQuicklistMenuitems.cpp	2012-03-14 06:24:18 +0000
+++ tests/unit/TestQuicklistMenuitems.cpp	2012-04-19 00:02:18 +0000
@@ -94,6 +94,7 @@
   g_assert_cmpstr(qlCheckmarkItem->GetLabel(), == , "Unchecked");
   g_assert_cmpint(qlCheckmarkItem->GetEnabled(), == , false);
   g_assert_cmpint(qlCheckmarkItem->GetActive(), == , false);
+  g_assert_cmpint(qlCheckmarkItem->GetSelectable(), == , false);
   g_assert_cmpint(qlCheckmarkItem->IsMarkupEnabled(), == , false);
 
   //qlCheckmarkItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
@@ -129,6 +130,7 @@
 
   g_assert_cmpstr(qlLabelItem->GetLabel(), == , "A Label");
   g_assert_cmpint(qlLabelItem->GetEnabled(), == , true);
+  g_assert_cmpint(qlLabelItem->GetSelectable(), == , true);
   g_assert_cmpint(qlLabelItem->IsMarkupEnabled(), == , true);
 
   //qlLabelItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
@@ -169,6 +171,7 @@
   g_assert_cmpstr(qlRadioItem->GetLabel(), == , "Radio Active");
   g_assert_cmpint(qlRadioItem->GetEnabled(), == , true);
   g_assert_cmpint(qlRadioItem->GetActive(), == , true);
+  g_assert_cmpint(qlRadioItem->GetSelectable(), == , true);
   g_assert_cmpint(qlRadioItem->IsMarkupEnabled(), == , true);
 
   //qlRadioItem->sigChanged.connect (sigc::mem_fun (pointerToCallerClass,
@@ -198,6 +201,7 @@
   qlSeparatorItem = new QuicklistMenuItemSeparator(item, true);
 
   g_assert_cmpint(qlSeparatorItem->GetEnabled(), == , true);
+  g_assert_cmpint(qlSeparatorItem->GetSelectable(), == , false);
 
   qlSeparatorItem->Dispose();
   g_object_unref(item);