Merge lp:~sil2100/unity/hud_icon_bfb_fix into lp:unity

[Łukasz Zemczak]

When opening the HUD when the Dash is opened when there are no applications, a '?' icon is shown instead of the BFB icon. To fix this, we ignore all Unity-related windows in the GetActiveWindow() function.

- Problem:

When there are no focused applications on the desktop, the dash is opened and then instantly the HUD, the HUD icon is wrong (showing '?'). It's because the dash is the currently active application then, so it tries to fetch the icon of the dash instead.

- Fix:

A quick workaround - when we show the HUD, we also make sure the active application is not the dash, basing on the application title.

- Test:

Already handled by existing test (test_dash_hud_only_uses_icon_from_current_desktop).

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

More than doing this in the HUD itself, probably it's just better to avoid to get an unity window as the active one in the Application manager (as we did before).

To do that just filter-out the xid's that are in nux::XInputWindow::NativeHandleList() in bamf::Manager::GetActiveWindow()

[Łukasz Zemczak (sil2100)]

Thanks, will try doing it this way. Actually this way was the easiest and fastest ;) So a better solution like this is a good idea.

[Łukasz Zemczak (sil2100)]

On the other hand, I also remembered why I preferred not to look into the GetActiveWindow() code - since I didn't want to break anything in the meantime. Since I didn't want to assume that the dash will be never fetched by GetActiveWindow() to actually do anything practical. Is it really safe to filter out the dash in this code? Or is there even a small chance that it should be fetch-able through GetActiveWindow()?

review: Needs Information

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

I think it's safe. Really no part of unity needs to get one of its window as active.

Since we're a shell, when it comes to window management, we should always act as we'd not exist.

[Łukasz Zemczak (sil2100)]

Modified!

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

32 + std::find(our_xids.begin(), our_xids.end(), active_xid) == our_xids.end())

I think you should check "xid" here instead of "active_xid" or you'll repeat the same check.

Otherwise only define "Window xid" at at above level, and always use it to avoid confusion.

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

Looks good now, thanks!

review: Approve

=== modified file 'unity-shared/BamfApplicationManager.cpp'
--- unity-shared/BamfApplicationManager.cpp	2012-12-18 17:57:48 +0000
+++ unity-shared/BamfApplicationManager.cpp	2013-01-08 12:21:22 +0000
@@ -21,6 +21,7 @@
 #include "unity-shared/WindowManager.h"
 
 #include <NuxCore/Logger.h>
+#include <NuxGraphics/XInputWindow.h>
 
 
 DECLARE_LOGGER(logger, "unity.appmanager.bamf");
@@ -545,6 +546,13 @@
   // No transfer of ownership for bamf_matcher_get_active_window.
   BamfWindow* active_win = bamf_matcher_get_active_window(matcher_);
 
+  std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
+  Window xid = bamf_window_get_xid(active_win);
+
+  // First check if the active window is not an Unity window
+  if (std::find(our_xids.begin(), our_xids.end(), xid) != our_xids.end())
+    active_win = nullptr;
+
   // If the active window is a dock type, then we want the first visible, non-dock type.
   if (active_win &&
       bamf_window_get_window_type(active_win) == BAMF_WINDOW_DOCK)
@@ -562,12 +570,13 @@
 
       auto win = static_cast<BamfWindow*>(l->data);
       auto view = static_cast<BamfView*>(l->data);
-      Window xid = bamf_window_get_xid(win);
+      xid = bamf_window_get_xid(win);
 
       if (bamf_view_is_user_visible(view) &&
           bamf_window_get_window_type(win) != BAMF_WINDOW_DOCK &&
           wm.IsWindowOnCurrentDesktop(xid) &&
-          wm.IsWindowVisible(xid))
+          wm.IsWindowVisible(xid) &&
+          std::find(our_xids.begin(), our_xids.end(), xid) == our_xids.end())
       {
         active_win = win;
       }