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

Proposed by Brandon Schaefer
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 2697
Merged at revision: 2745
Proposed branch: lp:~brandontschaefer/unity/ignore-override-redirect-in-window-on-top
Merge into: lp:unity
Diff against target: 127 lines (+54/-30)
3 files modified
tests/autopilot/unity/tests/launcher/test_icon_behavior.py (+27/-0)
unity-shared/PluginAdapter.h (+2/-0)
unity-shared/PluginAdapterCompiz.cpp (+25/-30)
To merge this branch: bzr merge lp:~brandontschaefer/unity/ignore-override-redirect-in-window-on-top
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Review via email: mp+124512@code.launchpad.net

Commit message

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

Description of the change

=== 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

To post a comment you must log in.
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

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 :)

2694. By Brandon Schaefer

* Adds a tests with an override redirect window

2695. By Brandon Schaefer

* Remove redundant code

2696. By Brandon Schaefer

* small change

2697. By Brandon Schaefer

* Check that the window is owned by nux.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Reviewed in IRC; looks good to me! ;)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
19from unity.emulators.launcher import IconDragType19from unity.emulators.launcher import IconDragType
20from unity.tests.launcher import LauncherTestCase, _make_scenarios20from unity.tests.launcher import LauncherTestCase, _make_scenarios
2121
22import pygtk
23import gtk
24
22logger = logging.getLogger(__name__)25logger = logging.getLogger(__name__)
2326
2427
@@ -114,6 +117,30 @@
114 self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))117 self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
115 self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))118 self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
116119
120 def test_clicking_icon_twice_with_override_redirect_window_opens_spread(self):
121 """This tests shows that when you click on a launcher icon twice and a
122 window exist that has override direct set then it must ignore that window
123 and initate spread.
124 """
125 # Opens an override redirect window
126 popup = gtk.Window(gtk.WINDOW_POPUP)
127 popup.show_all()
128 self.addCleanup(popup.destroy)
129
130 char_win1 = self.start_app_window("Character Map")
131 char_win2 = self.start_app_window("Character Map")
132 char_app = char_win1.application
133
134 self.assertVisibleWindowStack([char_win2, char_win1])
135 self.assertProperty(char_win2, is_focused=True)
136
137 char_icon = self.launcher.model.get_icon(desktop_id=char_app.desktop_file)
138 self.addCleanup(self.keybinding, "spread/cancel")
139 self.launcher_instance.click_launcher_icon(char_icon)
140
141 self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
142 self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
143
117 def test_while_in_scale_mode_the_dash_will_still_open(self):144 def test_while_in_scale_mode_the_dash_will_still_open(self):
118 """If scale is initiated through the laucher pressing super must close145 """If scale is initiated through the laucher pressing super must close
119 scale and open the dash.146 scale and open the dash.
120147
=== 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 @@
180 bool CheckWindowIntersection(nux::Geometry const& region, CompWindow* window) const;180 bool CheckWindowIntersection(nux::Geometry const& region, CompWindow* window) const;
181 void SetMwmWindowHints(Window xid, MotifWmHints* new_hints);181 void SetMwmWindowHints(Window xid, MotifWmHints* new_hints);
182182
183 Window GetTopMostValidWindowInViewport() const;
184
183 std::string GetTextProperty(guint32 xid, Atom atom) const;185 std::string GetTextProperty(guint32 xid, Atom atom) const;
184 std::string GetUtf8Property(guint32 xid, Atom atom) const;186 std::string GetUtf8Property(guint32 xid, Atom atom) const;
185187
186188
=== 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 @@
538bool538bool
539PluginAdapter::IsWindowOnTop(guint32 xid) const539PluginAdapter::IsWindowOnTop(guint32 xid) const
540{540{
541 Window win = xid;541 if (xid == GetTopMostValidWindowInViewport())
542 CompWindow* window = m_Screen->findWindow(win);542 return true;
543543
544 if (window)544 return false;
545}
546
547Window PluginAdapter::GetTopMostValidWindowInViewport() const
548{
549 CompWindow* window;
550 CompPoint screen_vp = m_Screen->vp();
551 std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
552
553 auto const& windows = m_Screen->windows();
554 for (auto it = windows.rbegin(); it != windows.rend(); ++it)
545 {555 {
546 if (window->inShowDesktopMode() || !window->isMapped() || !window->isViewable() || window->minimized())556 window = *it;
547 return false;557 if (window->defaultViewport() == screen_vp &&
548558 window->isViewable() && window->isMapped() &&
549 CompPoint window_vp = window->defaultViewport();559 !window->minimized() && !window->inShowDesktopMode() &&
550 nux::Geometry const& window_vp_geo = GetWorkAreaGeometry(window->id());560 !(window->state() & CompWindowStateAboveMask) &&
551 std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();561 !(window->type() & CompWindowTypeSplashMask) &&
552562 !(window->type() & CompWindowTypeDockMask) &&
553 for (CompWindow* sibling = window->next; sibling; sibling = sibling->next)563 !window->overrideRedirect() &&
564 std::find(our_xids.begin(), our_xids.end(), window) == our_xids.end())
554 {565 {
555 if (sibling->defaultViewport() == window_vp && !sibling->minimized() &&566 return window->id();
556 sibling->isMapped() && sibling->isViewable() && !sibling->inShowDesktopMode() &&
557 !(sibling->state() & CompWindowStateAboveMask) &&
558 !(sibling->type() & CompWindowTypeSplashMask) &&
559 !(sibling->type() & CompWindowTypeDockMask) &&
560 /* FIXME: This should be included by the above defaultViewport() check,
561 * but it doesn't seem to work correctly when there's only one workspace
562 * enabled, so please drop the line above when bug #996604 is fixed in
563 * Compiz. */
564 !window_vp_geo.Intersect(GetWindowGeometry(sibling->id())).IsNull() &&
565 std::find(our_xids.begin(), our_xids.end(), sibling->id()) == our_xids.end())
566 {
567 return false;
568 }
569 }567 }
570
571 return true;
572 }568 }
573569 return 0;
574 return false;
575}570}
576571
577bool572bool