Merge lp:~3v1n0/unity/quicklist-app-name-spread-fix into lp:unity

[Marco Trevisan (Treviño)]

BamfLauncherIcon: activate the icon using the quicklist item using an Idle.

This fixes the bug #986461 that caused the spread not to initiate clicking on the quicklist application name.

When the quicklist application name item is clicked the spread should eventually initiate (if the application is active and visible), but this doesn't seem to work anymore.

The problem seems to be due to the fact that the quicklist view grab is still active when the "item-activated" event happens, and so compiz doesn't seem to be able to initiate the scale plugin.

Adding an idle on the activate event fixes the issue.

Fix tested using Autopilot, added introspection support to the WindowManager and PluginAdapter classes.

[Thomi Richards (thomir-deactivatedaccount)]

Approved.

Probably want to get someone who knows the unity-side of things to look through this as well though.

review: Approve

=== modified file 'plugins/unityshell/src/BamfLauncherIcon.cpp'
--- plugins/unityshell/src/BamfLauncherIcon.cpp	2012-04-25 00:32:14 +0000
+++ plugins/unityshell/src/BamfLauncherIcon.cpp	2012-04-25 23:33:18 +0000
@@ -149,6 +149,9 @@
   if (_fill_supported_types_id != 0)
     g_source_remove(_fill_supported_types_id);
 
+  if (_quicklist_activated_id != 0)
+    g_source_remove(_quicklist_activated_id);
+
   if (_window_moved_id != 0)
     g_source_remove(_window_moved_id);
 
@@ -963,7 +966,13 @@
 
     _gsignals.Add(new glib::Signal<void, DbusmenuMenuitem*, int>(item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                                     [&] (DbusmenuMenuitem*, int) {
-                                      ActivateLauncherIcon(ActionArg(ActionArg::OTHER, 0));
+                                      _quicklist_activated_id =
+                                        g_idle_add([] (gpointer data) -> gboolean {
+                                          auto self = static_cast<BamfLauncherIcon*>(data);
+                                          self->ActivateLauncherIcon(ActionArg());
+                                          self->_quicklist_activated_id = 0;
+                                          return FALSE;
+                                        }, this);
                                     }));
 
     _menu_items_extra["AppName"] = glib::Object<DbusmenuMenuitem>(item);

=== modified file 'plugins/unityshell/src/BamfLauncherIcon.h'
--- plugins/unityshell/src/BamfLauncherIcon.h	2012-04-25 00:32:14 +0000
+++ plugins/unityshell/src/BamfLauncherIcon.h	2012-04-25 23:33:18 +0000
@@ -126,6 +126,7 @@
   bool _supported_types_filled;
   guint _fill_supported_types_id;
   guint _window_moved_id;
+  guint _quicklist_activated_id;
 
   std::string _remote_uri;
   std::string _desktop_file;

=== modified file 'plugins/unityshell/src/PluginAdapter.cpp'
--- plugins/unityshell/src/PluginAdapter.cpp	2012-04-20 02:47:02 +0000
+++ plugins/unityshell/src/PluginAdapter.cpp	2012-04-25 23:33:18 +0000
@@ -23,6 +23,7 @@
 #include "UScreen.h"
 
 #include <NuxCore/Logger.h>
+#include <UnityCore/Variant.h>
 
 namespace
 {
@@ -1266,3 +1267,16 @@
     _last_focused_window = NULL;
 }
 
+void
+PluginAdapter::AddProperties(GVariantBuilder* builder)
+{
+  unity::variant::BuilderWrapper wrapper(builder);
+  wrapper.add(GetScreenGeometry())
+         .add("workspace_count", WorkspaceCount())
+         .add("active_window", GetActiveWindow())
+         .add("screen_grabbed", IsScreenGrabbed())
+         .add("scale_active", IsScaleActive())
+         .add("scale_active_for_group", IsScaleActiveForGroup())
+         .add("expo_active", IsExpoActive())
+         .add("viewport_switch_running", IsViewPortSwitchStarted());
+}

=== modified file 'plugins/unityshell/src/PluginAdapter.h'
--- plugins/unityshell/src/PluginAdapter.h	2012-04-20 02:47:02 +0000
+++ plugins/unityshell/src/PluginAdapter.h	2012-04-25 23:33:18 +0000
@@ -165,6 +165,7 @@
 
 protected:
   PluginAdapter(CompScreen* screen);
+  void AddProperties(GVariantBuilder* builder);
 
 private:
   std::string MatchStringForXids(std::vector<Window> *windows);

=== modified file 'plugins/unityshell/src/WindowManager.cpp'
--- plugins/unityshell/src/WindowManager.cpp	2012-04-20 02:47:02 +0000
+++ plugins/unityshell/src/WindowManager.cpp	2012-04-25 23:33:18 +0000
@@ -214,6 +214,10 @@
   {
     g_debug("%s", G_STRFUNC);
   }
+
+  void AddProperties(GVariantBuilder* builder)
+  {
+  }
 };
 
 WindowManager*
@@ -231,6 +235,11 @@
   window_manager = manager;
 }
 
+std::string WindowManager::GetName() const
+{
+  return "WindowManager";
+}
+
 #define NET_WM_MOVERESIZE_MOVE 8
 
 void WindowManager::StartMove(guint32 xid, int x, int y)

=== modified file 'plugins/unityshell/src/WindowManager.h'
--- plugins/unityshell/src/WindowManager.h	2012-04-20 02:47:02 +0000
+++ plugins/unityshell/src/WindowManager.h	2012-04-25 23:33:18 +0000
@@ -19,24 +19,18 @@
 #ifndef WINDOW_MANAGER_H
 #define WINDOW_MANAGER_H
 
-#include <glib.h>
-#include <sigc++/sigc++.h>
 #include <Nux/Nux.h>
-#include <Nux/WindowThread.h>
-#include <NuxGraphics/GLWindowManager.h>
 #include <gdk/gdkx.h>
 #include <core/core.h>
 
-class WindowManager
+#include "Introspectable.h"
+
+class WindowManager : public unity::debug::Introspectable
 {
   // This is a glue interface that breaks the dependancy of Unity with Compiz
   // Basically it has a default implementation that does nothing useful, but
   // the idea is that unity.cpp uses SetDefault() early enough in it's
   // initialization so the things that require it get a usable implementation
-  //
-  // Currently only the Panel uses it but hopefully we'll get more of
-  // PluginAdaptor features moved into here and also get the Launcher to use
-  // it.
 
 public:
   WindowManager() :
@@ -142,6 +136,10 @@
 
   sigc::signal<void, const char*, const char*, CompOption::Vector&> compiz_event;
 
+protected:
+  std::string GetName() const;
+  virtual void AddProperties(GVariantBuilder* builder) = 0;
+
 private:
   Atom m_MoveResizeAtom;
 };

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-04-24 20:52:16 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-04-25 23:33:18 +0000
@@ -217,6 +217,7 @@
 
      PluginAdapter::Initialize(screen);
      WindowManager::SetDefault(PluginAdapter::Default());
+     AddChild(PluginAdapter::Default());
 
      StartupNotifyService::Default()->SetSnDisplay(screen->snDisplay(), screen->screenNum());
 
@@ -243,7 +244,6 @@
 
      wt->Run(NULL);
      uScreen = this;
-
      _in_paint = false;
 
 #ifndef USE_GLES

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2012-04-24 20:52:16 +0000
+++ plugins/unityshell/src/unityshell.h	2012-04-25 23:33:18 +0000
@@ -241,7 +241,7 @@
   internal::FavoriteStoreGSettings favorite_store_;
 
   /* The window thread should be the last thing removed, as c++ does it in reverse order */
-  std::unique_ptr<nux::WindowThread>    wt;
+  std::unique_ptr<nux::WindowThread> wt;
 
   /* These must stay below the window thread, please keep the order */
   launcher::Controller::Ptr launcher_controller_;
@@ -250,7 +250,7 @@
   switcher::Controller::Ptr switcher_controller_;
   hud::Controller::Ptr      hud_controller_;
   shortcut::Controller::Ptr shortcut_controller_;
-  debug::DebugDBusInterface  debugger_;
+  debug::DebugDBusInterface debugger_;
 
   std::list<shortcut::AbstractHint::Ptr> hints_;
   bool enable_shortcut_overlay_;

=== modified file 'tests/autopilot/autopilot/emulators/unity/quicklist.py'
--- tests/autopilot/autopilot/emulators/unity/quicklist.py	2012-04-18 23:01:46 +0000
+++ tests/autopilot/autopilot/emulators/unity/quicklist.py	2012-04-25 23:33:18 +0000
@@ -40,6 +40,10 @@
 
         return matches[0] if matches else None
 
+    def get_quicklist_application_item(self, application_name):
+        """Returns the QuicklistMenuItemLabel for the given application_name"""
+        return self.get_quicklist_item_by_text("<b>"+application_name+"</b>")
+
     def click_item(self, item):
         """Click one of the quicklist items."""
         if not isinstance(item, QuicklistMenuItem):

=== added file 'tests/autopilot/autopilot/emulators/unity/window_manager.py'
--- tests/autopilot/autopilot/emulators/unity/window_manager.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/autopilot/emulators/unity/window_manager.py	2012-04-25 23:33:18 +0000
@@ -0,0 +1,22 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+# Copyright 2012 Canonical
+# Author: 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 logging
+from autopilot.emulators.unity import UnityIntrospectionObject
+
+logger = logging.getLogger(__name__)
+
+
+class WindowManager(UnityIntrospectionObject):
+    """The WindowManager class."""
+
+    @property
+    def screen_geometry(self):
+        """Returns a tuple of (x,y,w,h) for the screen."""
+        return (self.x, self.y, self.width, self.height)

=== modified file 'tests/autopilot/autopilot/keybindings.py'
--- tests/autopilot/autopilot/keybindings.py	2012-04-11 23:54:13 +0000
+++ tests/autopilot/autopilot/keybindings.py	2012-04-25 23:33:18 +0000
@@ -112,6 +112,9 @@
     # expo plugin:
     "expo/start": ("expo", "expo_key"),
     "expo/cancel": "Escape",
+    # spread (scale) plugin:
+    "spread/start": ("scale", "initiate_all_key"),
+    "spread/cancel": "Escape",
 }
 
 

=== modified file 'tests/autopilot/autopilot/tests/__init__.py'
--- tests/autopilot/autopilot/tests/__init__.py	2012-04-17 13:34:40 +0000
+++ tests/autopilot/autopilot/tests/__init__.py	2012-04-25 23:33:18 +0000
@@ -27,6 +27,7 @@
 from autopilot.emulators.unity.launcher import LauncherController
 from autopilot.emulators.unity.panel import PanelController
 from autopilot.emulators.unity.switcher import Switcher
+from autopilot.emulators.unity.window_manager import WindowManager
 from autopilot.emulators.unity.workspace import WorkspaceManager
 from autopilot.emulators.X11 import ScreenGeometry, Keyboard, Mouse, reset_display
 from autopilot.glibrunner import GlibRunner
@@ -239,6 +240,7 @@
         self.launcher = self._get_launcher_controller()
         self.panels = self._get_panel_controller()
         self.switcher = Switcher()
+        self.window_manager = self._get_window_manager()
         self.workspace = WorkspaceManager()
         self.screen_geo = ScreenGeometry()
         self.addCleanup(self.workspace.switch_to, self.workspace.current_workspace)
@@ -336,6 +338,11 @@
         self.assertThat(len(controllers), Equals(1))
         return controllers[0]
 
+    def _get_window_manager(self):
+        managers = WindowManager.get_all_instances()
+        self.assertThat(len(managers), Equals(1))
+        return managers[0]
+
     def assertVisibleWindowStack(self, stack_start):
         """Check that the visible window stack starts with the windows passed in.
 

=== modified file 'tests/autopilot/autopilot/tests/test_quicklist.py'
--- tests/autopilot/autopilot/tests/test_quicklist.py	2012-04-18 23:58:19 +0000
+++ tests/autopilot/autopilot/tests/test_quicklist.py	2012-04-25 23:33:18 +0000
@@ -24,6 +24,11 @@
         ('remmina', {'app_name': 'Remmina'}),
     ]
 
+    def open_quicklist_for_icon(self, launcher_icon):
+        launcher = self.launcher.get_launcher_for_monitor(0)
+        launcher.click_launcher_icon(launcher_icon, button=3)
+        self.addCleanup(self.keyboard.press_and_release, "Escape")
+
     def test_quicklist_actions(self):
         """Test that all actions present in the destop file are shown in the quicklist."""
         self.start_app(self.app_name)
@@ -37,9 +42,7 @@
         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")
+        self.open_quicklist_for_icon(launcher_icon)
         ql = launcher_icon.get_quicklist()
         ql_item_texts = [i.text for i in ql.items if type(i) is QuicklistMenuItemLabel]
 
@@ -53,6 +56,82 @@
             name = de.content[key]['Name']
             self.assertThat(ql_item_texts, Contains(name))
 
+    def test_quicklist_application_item_focus_last_active_window(self):
+        """This tests shows that when you activate a quicklist application item
+        only the last focused instance of that application is rasied.
+
+        This is tested by opening 2 Mahjongg and a Calculator.
+        Then we activate the Calculator quicklist item.
+        Then we actiavte the Mahjongg launcher icon.
+        """
+        mahj = self.start_app("Mahjongg")
+        [mah_win1] = mahj.get_windows()
+        self.assertTrue(mah_win1.is_focused)
+
+        calc = self.start_app("Calculator")
+        [calc_win] = calc.get_windows()
+        self.assertTrue(calc_win.is_focused)
+
+        self.start_app("Mahjongg")
+        # Sleeping due to the start_app only waiting for the bamf model to be
+        # updated with the application.  Since the app has already started,
+        # and we are just waiting on a second window, however a defined sleep
+        # here is likely to be problematic.
+        # TODO: fix bamf emulator to enable waiting for new windows.
+        sleep(1)
+        [mah_win2] = [w for w in mahj.get_windows() if w.x_id != mah_win1.x_id]
+        self.assertTrue(mah_win2.is_focused)
+
+        self.assertVisibleWindowStack([mah_win2, calc_win, mah_win1])
+
+        mahj_icon = self.launcher.model.get_icon_by_desktop_id(mahj.desktop_file)
+        calc_icon = self.launcher.model.get_icon_by_desktop_id(calc.desktop_file)
+
+        self.open_quicklist_for_icon(calc_icon)
+        calc_ql = calc_icon.get_quicklist()
+        calc_ql.get_quicklist_application_item(calc.name).mouse_click()
+        sleep(1)
+        self.assertTrue(calc_win.is_focused)
+        self.assertVisibleWindowStack([calc_win, mah_win2, mah_win1])
+
+        self.open_quicklist_for_icon(mahj_icon)
+        mahj_ql = mahj_icon.get_quicklist()
+        mahj_ql.get_quicklist_application_item(mahj.name).mouse_click()
+        sleep(1)
+        self.assertTrue(mah_win2.is_focused)
+        self.assertVisibleWindowStack([mah_win2, calc_win, mah_win1])
+
+    def test_quicklist_application_item_initiate_spread(self):
+        """This tests shows that when you activate a quicklist application item
+        when an application window is focused, the spread is initiated.
+        """
+        calc = self.start_app("Calculator")
+        [calc_win1] = calc.get_windows()
+        self.assertTrue(calc_win1.is_focused)
+
+        self.start_app("Calculator")
+        # Sleeping due to the start_app only waiting for the bamf model to be
+        # updated with the application.  Since the app has already started,
+        # and we are just waiting on a second window, however a defined sleep
+        # here is likely to be problematic.
+        # TODO: fix bamf emulator to enable waiting for new windows.
+        sleep(1)
+        [calc_win2] = [w for w in calc.get_windows() if w.x_id != calc_win1.x_id]
+
+        self.assertVisibleWindowStack([calc_win2, calc_win1])
+        self.assertTrue(calc_win2.is_focused)
+
+        calc_icon = self.launcher.model.get_icon_by_desktop_id(calc.desktop_file)
+
+        self.open_quicklist_for_icon(calc_icon)
+        calc_ql = calc_icon.get_quicklist()
+        app_item = calc_ql.get_quicklist_application_item(calc.name)
+
+        self.addCleanup(self.keybinding, "spread/cancel")
+        app_item.mouse_click()
+        self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
+        self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
+
 
 class QuicklistKeyNavigationTests(AutopilotTestCase):
     """Tests for the quicklist key navigation."""