Merge lp:~brandontschaefer/unity/ignore-override-redirect-in-window-on-top into lp:unity

[Brandon Schaefer]

Ignore windows that have Override Redirect set to true when checking if a window is on top.

=== Problem ===
The Ibus language bar is a gtk.WINDOW_POPUP which means it is ignored by the WM for stacking. This means it was staying on the top of all the windows, while not being able to tell if it was or not. In PluginAdapterCompiz::IsWindowOnTop

=== Fix ===
Ignore any window that has override redirect set to true. I also re-factored the code a bit. Instead of going through all the siblings of a window we start that top of the stacking and grab the first valid one.

=== Test ===
AP test coming

[Brandon Schaefer (brandontschaefer)]

I also could not reproduce this issue anymore:

59 - /* FIXME: This should be included by the above defaultViewport() check,
60 - * but it doesn't seem to work correctly when there's only one workspace
61 - * enabled, so please drop the line above when bug #996604 is fixed in
62 - * Compiz. */

If anyone is able to double check that as well that would be awesome :)

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

Reviewed in IRC; looks good to me! ;)

review: Approve

=== modified file 'tests/autopilot/unity/tests/launcher/test_icon_behavior.py'
--- tests/autopilot/unity/tests/launcher/test_icon_behavior.py	2012-09-17 20:50:20 +0000
+++ tests/autopilot/unity/tests/launcher/test_icon_behavior.py	2012-09-20 23:31:23 +0000
@@ -19,6 +19,9 @@
 from unity.emulators.launcher import IconDragType
 from unity.tests.launcher import LauncherTestCase, _make_scenarios
 
+import pygtk
+import gtk
+
 logger = logging.getLogger(__name__)
 
 
@@ -114,6 +117,30 @@
         self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
         self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
 
+    def test_clicking_icon_twice_with_override_redirect_window_opens_spread(self):
+        """This tests shows that when you click on a launcher icon twice and a
+        window exist that has override direct set then it must ignore that window
+        and initate spread.
+        """
+        # Opens an override redirect window
+        popup = gtk.Window(gtk.WINDOW_POPUP)
+        popup.show_all()
+        self.addCleanup(popup.destroy)
+
+        char_win1 = self.start_app_window("Character Map")
+        char_win2 = self.start_app_window("Character Map")
+        char_app = char_win1.application
+
+        self.assertVisibleWindowStack([char_win2, char_win1])
+        self.assertProperty(char_win2, is_focused=True)
+
+        char_icon = self.launcher.model.get_icon(desktop_id=char_app.desktop_file)
+        self.addCleanup(self.keybinding, "spread/cancel")
+        self.launcher_instance.click_launcher_icon(char_icon)
+
+        self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
+        self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
+
     def test_while_in_scale_mode_the_dash_will_still_open(self):
         """If scale is initiated through the laucher pressing super must close
         scale and open the dash.

=== modified file 'unity-shared/PluginAdapter.h'
--- unity-shared/PluginAdapter.h	2012-09-10 22:44:29 +0000
+++ unity-shared/PluginAdapter.h	2012-09-20 23:31:23 +0000
@@ -180,6 +180,8 @@
   bool CheckWindowIntersection(nux::Geometry const& region, CompWindow* window) const;
   void SetMwmWindowHints(Window xid, MotifWmHints* new_hints);
 
+  Window GetTopMostValidWindowInViewport() const;
+
   std::string GetTextProperty(guint32 xid, Atom atom) const;
   std::string GetUtf8Property(guint32 xid, Atom atom) const;
 

=== modified file 'unity-shared/PluginAdapterCompiz.cpp'
--- unity-shared/PluginAdapterCompiz.cpp	2012-09-10 22:44:29 +0000
+++ unity-shared/PluginAdapterCompiz.cpp	2012-09-20 23:31:23 +0000
@@ -538,40 +538,35 @@
 bool
 PluginAdapter::IsWindowOnTop(guint32 xid) const
 {
-  Window win = xid;
-  CompWindow* window = m_Screen->findWindow(win);
-
-  if (window)
+  if (xid == GetTopMostValidWindowInViewport())
+    return true;
+
+  return false;
+}
+
+Window PluginAdapter::GetTopMostValidWindowInViewport() const
+{
+  CompWindow* window;
+  CompPoint screen_vp = m_Screen->vp();
+  std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
+
+  auto const& windows = m_Screen->windows();
+  for (auto it = windows.rbegin(); it != windows.rend(); ++it)
   {
-    if (window->inShowDesktopMode() || !window->isMapped() || !window->isViewable() || window->minimized())
-      return false;
-
-    CompPoint window_vp = window->defaultViewport();
-    nux::Geometry const& window_vp_geo = GetWorkAreaGeometry(window->id());
-    std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
-
-    for (CompWindow* sibling = window->next; sibling; sibling = sibling->next)
+    window = *it;
+    if (window->defaultViewport() == screen_vp &&
+        window->isViewable() && window->isMapped() &&
+        !window->minimized() && !window->inShowDesktopMode() &&
+        !(window->state() & CompWindowStateAboveMask) &&
+        !(window->type() & CompWindowTypeSplashMask) &&
+        !(window->type() & CompWindowTypeDockMask) &&
+        !window->overrideRedirect() &&
+        std::find(our_xids.begin(), our_xids.end(), window) == our_xids.end())
     {
-      if (sibling->defaultViewport() == window_vp && !sibling->minimized() &&
-          sibling->isMapped() && sibling->isViewable() && !sibling->inShowDesktopMode() &&
-          !(sibling->state() & CompWindowStateAboveMask) &&
-          !(sibling->type() & CompWindowTypeSplashMask) &&
-          !(sibling->type() & CompWindowTypeDockMask) &&
-          /* FIXME: This should be included by the above defaultViewport() check,
-           * but it doesn't seem to work correctly when there's only one workspace
-           * enabled, so please drop the line above when bug #996604 is fixed in
-           * Compiz. */
-          !window_vp_geo.Intersect(GetWindowGeometry(sibling->id())).IsNull() &&
-          std::find(our_xids.begin(), our_xids.end(), sibling->id()) == our_xids.end())
-      {
-        return false;
-      }
+      return window->id();
     }
-
-    return true;
   }
-
-  return false;
+  return 0;
 }
 
 bool