Merge lp:~3v1n0/unity/focused-non-top-windows-fix into lp:unity

[Marco Trevisan (Treviño)]

BamfLauncherIcon: Focus an application if no child window is actually on top of the stack. Raises all urgent windows.

This fixes bug #802816 that happens when an application raises a window on the top of the stack but the focus doesn't change.

Plus, don't perform unneeded checks when focusing non-active applications.

When a launcher icon is clicked, it should focus a window if the window is currently not on top of the stack (excluding from that the unity or other invalid windows).
To do that I've implemented a new method into PluginAdapter that checks if a Window is on top.

Also, when activating a launcher icon, we should always raise all the urgent windows, not only the top one.

Added manual tests, due to the fact that AP at the moment can't create such kind of windows on the fly.

[Alex Launi (alexlauni)]

This is autopilot testable. Start gimp, execute gimp foo-image.png (we'll need to ship an image to test with). Then check state. Simple.

review: Needs Fixing (quality)

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

Alex, I can't get this into an AP test... It fails to work like it should.
Also it's too application dependent and not always reproducible, so we must go with manual test.

[Tim Penhey (thumper)]

I'm ok with a manual test for this at this stage.

review: Approve

[Unity Merger (unity-merger)]

Attempt to merge into lp:unity failed due to conflicts:

text conflict in manual-tests/Launcher.txt

=== modified file 'manual-tests/Launcher.txt'
--- manual-tests/Launcher.txt	2012-04-26 04:58:58 +0000
+++ manual-tests/Launcher.txt	2012-04-26 06:00:26 +0000
@@ -489,6 +489,7 @@
 The Firefox icon should *not* be highlighted during the drag&drop.
 Firefox should open the folder in a new Firefox window.
 
+
 Test Launcher with Chromium Web apps
 ------------------------------------
 This tests shows the integration of unity with the Chromium Web Apps
@@ -517,3 +518,25 @@
    application icon.
  * Clicking the "Home | Ubuntu" will activate its window, while clicking
    in the chromium icon will activate a chromium window.
+
+
+Test Launcher raises focused non-top windows
+--------------------------------------------
+This tests shows how the focused windows should be raised to the top of the
+stack, when another non-active window is over them.
+
+Actions:
+ * Open a terminal Window
+ * Open The GIMP
+ * In GIMP load an image, say /usr/share/backgrounds/warty-final-ubuntu.png
+ * From the terminal window run the command "gimp foo-image.png"
+
+Expected Results:
+ * A gimp dialog will be raised (so the terminal window is below it), but not focused
+ * Terminal launcher icon must have a filled-in arrow on the right of the icon
+
+Actions:
+ * Click on the launcher terminal icon
+
+Expected Results:
+ * Terminal window will be raised to the top of the stack (over the GIMP window)

=== modified file 'plugins/unityshell/src/BamfLauncherIcon.cpp'
--- plugins/unityshell/src/BamfLauncherIcon.cpp	2012-04-26 00:35:37 +0000
+++ plugins/unityshell/src/BamfLauncherIcon.cpp	2012-04-26 06:00:26 +0000
@@ -208,50 +208,59 @@
     auto bamf_view = glib::object_cast<BamfView>(_bamf_app);
     user_visible = bamf_view_user_visible(bamf_view);
 
-    bool any_visible = false;
-    bool any_mapped = false;
-    bool any_on_monitor = (arg.monitor < 0);
-    int active_monitor = arg.monitor;
-    GList* children = bamf_view_get_children(bamf_view);
-
-    for (GList* l = children; l; l = l->next)
+    if (active)
     {
-      if (!BAMF_IS_WINDOW(l->data))
-        continue;
-
-      auto view = static_cast<BamfView*>(l->data);
-      auto win = static_cast<BamfWindow*>(l->data);
-      Window xid = bamf_window_get_xid(win);
-
-      if (!any_visible && wm->IsWindowOnCurrentDesktop(xid))
-      {
-        any_visible = true;
-      }
-
-      if (!any_mapped && wm->IsWindowMapped(xid))
-      {
-        any_mapped = true;
-      }
-
-      if (!any_on_monitor && bamf_window_get_monitor(win) == arg.monitor &&
-          wm->IsWindowMapped(xid) && wm->IsWindowVisible(xid))
-      {
-        any_on_monitor = true;
-      }
-
-      if (bamf_view_is_active(view))
-      {
-        active_monitor = bamf_window_get_monitor(win);
-      }
+      bool any_visible = false;
+      bool any_mapped = false;
+      bool any_on_top = false;
+      bool any_on_monitor = (arg.monitor < 0);
+      int active_monitor = arg.monitor;
+      GList* children = bamf_view_get_children(bamf_view);
+
+      for (GList* l = children; l; l = l->next)
+      {
+        if (!BAMF_IS_WINDOW(l->data))
+          continue;
+
+        auto view = static_cast<BamfView*>(l->data);
+        auto win = static_cast<BamfWindow*>(l->data);
+        Window xid = bamf_window_get_xid(win);
+
+        if (!any_visible && wm->IsWindowOnCurrentDesktop(xid))
+        {
+          any_visible = true;
+        }
+
+        if (!any_mapped && wm->IsWindowMapped(xid))
+        {
+          any_mapped = true;
+        }
+
+        if (!any_on_top && wm->IsWindowOnTop(xid))
+        {
+          any_on_top = true;
+        }
+
+        if (!any_on_monitor && bamf_window_get_monitor(win) == arg.monitor &&
+            wm->IsWindowMapped(xid) && wm->IsWindowVisible(xid))
+        {
+          any_on_monitor = true;
+        }
+
+        if (bamf_view_is_active(view))
+        {
+          active_monitor = bamf_window_get_monitor(win);
+        }
+      }
+
+      g_list_free(children);
+
+      if (!any_visible || !any_mapped || !any_on_top)
+        active = false;
+
+      if (any_on_monitor && arg.monitor >= 0 && active_monitor != arg.monitor)
+        active = false;
     }
-
-    g_list_free(children);
-
-    if (!any_visible || !any_mapped)
-      active = false;
-
-    if (any_on_monitor && arg.monitor >= 0 && active_monitor != arg.monitor)
-      active = false;
   }
 
   /* Behaviour:
@@ -635,7 +644,8 @@
     }
   }
 
-  wm->FocusWindowGroup(windows, visibility, arg.monitor);
+  bool only_top_win = !any_urgent;
+  wm->FocusWindowGroup(windows, visibility, arg.monitor, only_top_win);
 }
 
 bool BamfLauncherIcon::Spread(bool current_desktop, int state, bool force)

=== modified file 'plugins/unityshell/src/PluginAdapter.cpp'
--- plugins/unityshell/src/PluginAdapter.cpp	2012-04-24 21:26:30 +0000
+++ plugins/unityshell/src/PluginAdapter.cpp	2012-04-26 06:00:26 +0000
@@ -530,6 +530,39 @@
 }
 
 bool
+PluginAdapter::IsWindowOnTop(guint32 xid)
+{
+  Window win = xid;
+  CompWindow* window;
+
+  window = m_Screen->findWindow(win);
+
+  if (window)
+  {
+    if (window->inShowDesktopMode() || !window->isMapped() || !window->isViewable() || window->minimized())
+      return false;
+
+    CompPoint window_vp = window->defaultViewport();
+    std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
+
+    for (CompWindow* sibling = window->next; sibling; sibling = sibling->next)
+    {
+      if (sibling->defaultViewport() == window_vp && !sibling->minimized() &&
+          sibling->isMapped() && sibling->isViewable() && !sibling->inShowDesktopMode() &&
+          !(sibling->state() & CompWindowStateAboveMask) &&
+          std::find(our_xids.begin(), our_xids.end(), sibling->id()) == our_xids.end())
+      {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  return false;
+}
+
+bool
 PluginAdapter::IsWindowClosable(guint32 xid)
 {
   Window win = xid;

=== modified file 'plugins/unityshell/src/PluginAdapter.h'
--- plugins/unityshell/src/PluginAdapter.h	2012-04-24 21:26:30 +0000
+++ plugins/unityshell/src/PluginAdapter.h	2012-04-26 06:00:26 +0000
@@ -121,6 +121,7 @@
   bool IsWindowObscured(guint xid);
   bool IsWindowMapped(guint xid);
   bool IsWindowVisible(guint32 xid);
+  bool IsWindowOnTop(guint32 xid);
   bool IsWindowClosable(guint32 xid);
   bool IsWindowMinimizable(guint32 xid);
   bool IsWindowMaximizable(guint32 xid);

=== modified file 'plugins/unityshell/src/WindowManager.cpp'
--- plugins/unityshell/src/WindowManager.cpp	2012-04-25 23:30:51 +0000
+++ plugins/unityshell/src/WindowManager.cpp	2012-04-26 06:00:26 +0000
@@ -78,6 +78,11 @@
     return true;
   }
 
+  bool IsWindowOnTop(guint32 xid)
+  {
+    return false;
+  }
+
   bool IsWindowClosable(guint32 xid)
   {
     return true;

=== modified file 'plugins/unityshell/src/WindowManager.h'
--- plugins/unityshell/src/WindowManager.h	2012-04-24 21:26:30 +0000
+++ plugins/unityshell/src/WindowManager.h	2012-04-26 06:00:26 +0000
@@ -57,6 +57,7 @@
   virtual bool IsWindowObscured(guint32 xid) = 0;
   virtual bool IsWindowMapped(guint32 xid) = 0;
   virtual bool IsWindowVisible(guint32 xid) = 0;
+  virtual bool IsWindowOnTop(guint32 xid) = 0;
   virtual bool IsWindowClosable(guint32 xid) = 0;
   virtual bool IsWindowMinimizable(guint32 xid) = 0;
   virtual bool IsWindowMaximizable(guint32 xid) = 0;