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
1=== modified file 'tests/autopilot/unity/tests/launcher/test_icon_behavior.py'
2--- tests/autopilot/unity/tests/launcher/test_icon_behavior.py 2012-09-17 20:50:20 +0000
3+++ tests/autopilot/unity/tests/launcher/test_icon_behavior.py 2012-09-20 23:31:23 +0000
4@@ -19,6 +19,9 @@
5 from unity.emulators.launcher import IconDragType
6 from unity.tests.launcher import LauncherTestCase, _make_scenarios
7
8+import pygtk
9+import gtk
10+
11 logger = logging.getLogger(__name__)
12
13
14@@ -114,6 +117,30 @@
15 self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
16 self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
17
18+ def test_clicking_icon_twice_with_override_redirect_window_opens_spread(self):
19+ """This tests shows that when you click on a launcher icon twice and a
20+ window exist that has override direct set then it must ignore that window
21+ and initate spread.
22+ """
23+ # Opens an override redirect window
24+ popup = gtk.Window(gtk.WINDOW_POPUP)
25+ popup.show_all()
26+ self.addCleanup(popup.destroy)
27+
28+ char_win1 = self.start_app_window("Character Map")
29+ char_win2 = self.start_app_window("Character Map")
30+ char_app = char_win1.application
31+
32+ self.assertVisibleWindowStack([char_win2, char_win1])
33+ self.assertProperty(char_win2, is_focused=True)
34+
35+ char_icon = self.launcher.model.get_icon(desktop_id=char_app.desktop_file)
36+ self.addCleanup(self.keybinding, "spread/cancel")
37+ self.launcher_instance.click_launcher_icon(char_icon)
38+
39+ self.assertThat(self.window_manager.scale_active, Eventually(Equals(True)))
40+ self.assertThat(self.window_manager.scale_active_for_group, Eventually(Equals(True)))
41+
42 def test_while_in_scale_mode_the_dash_will_still_open(self):
43 """If scale is initiated through the laucher pressing super must close
44 scale and open the dash.
45
46=== modified file 'unity-shared/PluginAdapter.h'
47--- unity-shared/PluginAdapter.h 2012-09-10 22:44:29 +0000
48+++ unity-shared/PluginAdapter.h 2012-09-20 23:31:23 +0000
49@@ -180,6 +180,8 @@
50 bool CheckWindowIntersection(nux::Geometry const& region, CompWindow* window) const;
51 void SetMwmWindowHints(Window xid, MotifWmHints* new_hints);
52
53+ Window GetTopMostValidWindowInViewport() const;
54+
55 std::string GetTextProperty(guint32 xid, Atom atom) const;
56 std::string GetUtf8Property(guint32 xid, Atom atom) const;
57
58
59=== modified file 'unity-shared/PluginAdapterCompiz.cpp'
60--- unity-shared/PluginAdapterCompiz.cpp 2012-09-10 22:44:29 +0000
61+++ unity-shared/PluginAdapterCompiz.cpp 2012-09-20 23:31:23 +0000
62@@ -538,40 +538,35 @@
63 bool
64 PluginAdapter::IsWindowOnTop(guint32 xid) const
65 {
66- Window win = xid;
67- CompWindow* window = m_Screen->findWindow(win);
68-
69- if (window)
70+ if (xid == GetTopMostValidWindowInViewport())
71+ return true;
72+
73+ return false;
74+}
75+
76+Window PluginAdapter::GetTopMostValidWindowInViewport() const
77+{
78+ CompWindow* window;
79+ CompPoint screen_vp = m_Screen->vp();
80+ std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
81+
82+ auto const& windows = m_Screen->windows();
83+ for (auto it = windows.rbegin(); it != windows.rend(); ++it)
84 {
85- if (window->inShowDesktopMode() || !window->isMapped() || !window->isViewable() || window->minimized())
86- return false;
87-
88- CompPoint window_vp = window->defaultViewport();
89- nux::Geometry const& window_vp_geo = GetWorkAreaGeometry(window->id());
90- std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
91-
92- for (CompWindow* sibling = window->next; sibling; sibling = sibling->next)
93+ window = *it;
94+ if (window->defaultViewport() == screen_vp &&
95+ window->isViewable() && window->isMapped() &&
96+ !window->minimized() && !window->inShowDesktopMode() &&
97+ !(window->state() & CompWindowStateAboveMask) &&
98+ !(window->type() & CompWindowTypeSplashMask) &&
99+ !(window->type() & CompWindowTypeDockMask) &&
100+ !window->overrideRedirect() &&
101+ std::find(our_xids.begin(), our_xids.end(), window) == our_xids.end())
102 {
103- if (sibling->defaultViewport() == window_vp && !sibling->minimized() &&
104- sibling->isMapped() && sibling->isViewable() && !sibling->inShowDesktopMode() &&
105- !(sibling->state() & CompWindowStateAboveMask) &&
106- !(sibling->type() & CompWindowTypeSplashMask) &&
107- !(sibling->type() & CompWindowTypeDockMask) &&
108- /* FIXME: This should be included by the above defaultViewport() check,
109- * but it doesn't seem to work correctly when there's only one workspace
110- * enabled, so please drop the line above when bug #996604 is fixed in
111- * Compiz. */
112- !window_vp_geo.Intersect(GetWindowGeometry(sibling->id())).IsNull() &&
113- std::find(our_xids.begin(), our_xids.end(), sibling->id()) == our_xids.end())
114- {
115- return false;
116- }
117+ return window->id();
118 }
119-
120- return true;
121 }
122-
123- return false;
124+ return 0;
125 }
126
127 bool