Merge lp:~brandontschaefer/unity/switcher-alt+f4-fix into lp:unity

Proposed by Brandon Schaefer
Status: Merged
Approved by: Brandon Schaefer
Approved revision: no longer in the source branch.
Merged at revision: 2441
Proposed branch: lp:~brandontschaefer/unity/switcher-alt+f4-fix
Merge into: lp:unity
Diff against target: 202 lines (+69/-4)
7 files modified
launcher/SwitcherController.cpp (+13/-0)
launcher/SwitcherController.h (+2/-0)
launcher/SwitcherModel.cpp (+1/-1)
plugins/unityshell/src/unityshell.cpp (+27/-0)
plugins/unityshell/src/unityshell.h (+5/-0)
tests/autopilot/unity/tests/test_switcher.py (+18/-2)
unity-shared/UBusMessages.h (+3/-1)
To merge this branch: bzr merge lp:~brandontschaefer/unity/switcher-alt+f4-fix
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Approve
Thomi Richards (community) quality Approve
Review via email: mp+111703@code.launchpad.net

Commit message

The switcher now focuses it's own window, instead of leaving it to the current one. Now Alt+F4 does not work while the switcher is active.

Description of the change

=== Problem ===
When the switcher starts it does not take the focus from the current window. This means if you press Alt+F4 it will close that program, but when you exit from the switcher it will restart that application.

=== Fix ===
The fix is to take the switchers view window and give it focus. If the switcher is cancled then restore focus to the previous window. Else, do what the switcher normally does.

=== Test ===
AP test included.

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

All switcher AP test pass locally.

Ran 24 tests in 185.487s

OK (skipped=1)

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

171 + self.keyboard.press_and_release("Alt+F4")
172 + [win] = [w for w in app.get_windows()]
173 +
174 + self.assertThat(win.is_valid, Equals(True))

This will also pass even if the code is broken and we run it on the jenkins machine. It's a bit ugly, but I'd like to see a "sleep(10)" just before the assert, with a comment saying why we need it.

Otherwise looks good to me.

Cheers

review: Needs Fixing (quality)
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) :
review: Approve (quality)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/SwitcherController.cpp'
2--- launcher/SwitcherController.cpp 2012-06-15 02:03:31 +0000
3+++ launcher/SwitcherController.cpp 2012-06-25 21:40:25 +0000
4@@ -159,8 +159,11 @@
5
6 ConstructView();
7
8+ ubus_manager_.SendMessage(UBUS_SWITCHER_START, NULL);
9+
10 if (view_window_) {
11 view_window_->ShowWindow(true);
12+ view_window_->PushToFront();
13 view_window_->SetOpacity(1.0f);
14 }
15 }
16@@ -179,6 +182,7 @@
17 view_window_->SetLayout(main_layout_);
18 view_window_->SetBackgroundColor(nux::Color(0x00000000));
19 view_window_->SetGeometry(workarea_);
20+ view_window_->EnableInputWindow(true, "Switcher", false, false);
21 }
22 }
23
24@@ -240,6 +244,8 @@
25 }
26 }
27
28+ ubus_manager_.SendMessage(UBUS_SWITCHER_END, g_variant_new_boolean(!accept_state));
29+
30 sources_.Remove(VIEW_CONSTRUCT_IDLE);
31 sources_.Remove(SHOW_TIMEOUT);
32 sources_.Remove(DETAIL_TIMEOUT);
33@@ -254,6 +260,8 @@
34 {
35 view_window_->SetOpacity(0.0f);
36 view_window_->ShowWindow(false);
37+ view_window_->PushToBack();
38+ view_window_->EnableInputWindow(false);
39 }
40
41 ubus_manager_.SendMessage(UBUS_SWITCHER_SHOWN, g_variant_new("(bi)", false, monitor_));
42@@ -385,6 +393,11 @@
43 return view_->ExternalTargets();
44 }
45
46+guint Controller::GetSwitcherInputWindowId() const
47+{
48+ return view_window_->GetInputWindowId();
49+}
50+
51 bool Controller::CompareSwitcherItemsPriority(AbstractLauncherIcon::Ptr first,
52 AbstractLauncherIcon::Ptr second)
53 {
54
55=== modified file 'launcher/SwitcherController.h'
56--- launcher/SwitcherController.h 2012-06-15 02:03:31 +0000
57+++ launcher/SwitcherController.h 2012-06-25 21:40:25 +0000
58@@ -90,6 +90,8 @@
59
60 ui::LayoutWindowList ExternalRenderTargets ();
61
62+ guint GetSwitcherInputWindowId() const;
63+
64 protected:
65 // Introspectable methods
66 std::string GetName() const;
67
68=== modified file 'launcher/SwitcherModel.cpp'
69--- launcher/SwitcherModel.cpp 2012-06-01 02:38:55 +0000
70+++ launcher/SwitcherModel.cpp 2012-06-25 21:40:25 +0000
71@@ -171,7 +171,7 @@
72
73 if (detail_selection_index > DetailXids().size() - 1)
74 return 0;
75-
76+
77 return DetailXids()[detail_selection_index];
78 }
79
80
81=== modified file 'plugins/unityshell/src/unityshell.cpp'
82--- plugins/unityshell/src/unityshell.cpp 2012-06-15 02:03:31 +0000
83+++ plugins/unityshell/src/unityshell.cpp 2012-06-25 21:40:25 +0000
84@@ -353,6 +353,12 @@
85 ubus_manager_.RegisterInterest(UBUS_LAUNCHER_END_KEY_SWTICHER,
86 sigc::mem_fun(this, &UnityScreen::OnLauncherEndKeyNav));
87
88+ ubus_manager_.RegisterInterest(UBUS_SWITCHER_START,
89+ sigc::mem_fun(this, &UnityScreen::OnSwitcherStart));
90+
91+ ubus_manager_.RegisterInterest(UBUS_SWITCHER_END,
92+ sigc::mem_fun(this, &UnityScreen::OnSwitcherEnd));
93+
94 auto init_plugins_cb = sigc::mem_fun(this, &UnityScreen::initPluginActions);
95 sources_.Add(std::make_shared<glib::Idle>(init_plugins_cb, glib::Source::Priority::DEFAULT));
96
97@@ -1909,6 +1915,27 @@
98
99 void UnityScreen::OnLauncherEndKeyNav(GVariant* data)
100 {
101+ RestoreWindow(data);
102+}
103+
104+void UnityScreen::OnSwitcherStart(GVariant* data)
105+{
106+ newFocusedWindow = screen->findWindow(switcher_controller_->GetSwitcherInputWindowId());
107+
108+ if (switcher_controller_->GetSwitcherInputWindowId() != screen->activeWindow())
109+ PluginAdapter::Default()->saveInputFocus();
110+
111+ if (newFocusedWindow)
112+ newFocusedWindow->moveInputFocusTo();
113+}
114+
115+void UnityScreen::OnSwitcherEnd(GVariant* data)
116+{
117+ RestoreWindow(data);
118+}
119+
120+void UnityScreen::RestoreWindow(GVariant* data)
121+{
122 bool preserve_focus = false;
123
124 if (data)
125
126=== modified file 'plugins/unityshell/src/unityshell.h'
127--- plugins/unityshell/src/unityshell.h 2012-06-15 02:03:31 +0000
128+++ plugins/unityshell/src/unityshell.h 2012-06-25 21:40:25 +0000
129@@ -228,6 +228,11 @@
130 void OnLauncherStartKeyNav(GVariant* data);
131 void OnLauncherEndKeyNav(GVariant* data);
132
133+ void OnSwitcherStart(GVariant* data);
134+ void OnSwitcherEnd(GVariant* data);
135+
136+ void RestoreWindow(GVariant* data);
137+
138 void InitHints();
139
140 void OnPanelStyleChanged();
141
142=== modified file 'tests/autopilot/unity/tests/test_switcher.py'
143--- tests/autopilot/unity/tests/test_switcher.py 2012-06-20 21:39:36 +0000
144+++ tests/autopilot/unity/tests/test_switcher.py 2012-06-25 21:40:25 +0000
145@@ -52,10 +52,10 @@
146 """Starting switcher in details mode must show the focused window title."""
147 app = self.start_app("Text Editor")
148 sleep(1)
149- self.switcher.initiate(SwitcherMode.DETAIL)
150- self.addCleanup(self.switcher.terminate)
151
152 [title] = [w.title for w in app.get_windows() if w.is_focused]
153+ self.switcher.initiate(SwitcherMode.DETAIL)
154+ self.addCleanup(self.switcher.terminate)
155
156 self.assertThat(self.switcher.controller.view.label, Eventually(Equals(title)))
157
158@@ -190,6 +190,22 @@
159 self.addCleanup(self.switcher.terminate)
160 self.assertThat(self.switcher.controller.monitor, Eventually(Equals(monitor)))
161
162+ def test_switcher_alt_f4_is_disabled(self):
163+ """Tests that alt+f4 does not work while switcher is active."""
164+
165+ app = self.start_app("Text Editor")
166+ sleep(1)
167+
168+ self.switcher.initiate(SwitcherMode.DETAIL)
169+ self.addCleanup(self.switcher.terminate)
170+
171+ self.keyboard.press_and_release("Alt+F4")
172+ [win] = [w for w in app.get_windows()]
173+
174+ # Need the sleep to allow the window time to close, for jenkins!
175+ sleep(10)
176+ self.assertThat(win.is_valid, Equals(True))
177+
178
179 class SwitcherWindowsManagementTests(SwitcherTestCase):
180 """Test the switcher window management."""
181
182=== modified file 'unity-shared/UBusMessages.h'
183--- unity-shared/UBusMessages.h 2012-05-06 23:48:38 +0000
184+++ unity-shared/UBusMessages.h 2012-06-25 21:40:25 +0000
185@@ -35,7 +35,7 @@
186 #define UBUS_DASH_ABOUT_TO_SHOW "DASH_ABOUT_TO_SHOW"
187
188 // Signal sent when an overlay interface is shown, includes a gvariant
189-// gvariant format is (sb), (interface-name, can_maximize?)
190+// gvariant format is (sb), (interface-name, can_maximize?)
191 #define UBUS_OVERLAY_FORMAT_STRING "(sbi)"
192 #define UBUS_OVERLAY_HIDDEN "OVERLAY_HIDDEN"
193 #define UBUS_OVERLAY_SHOWN "OVERLAY_SHOWN"
194@@ -88,6 +88,8 @@
195
196 // Signals sent when the switcher is shown, hidden or changes selection
197 #define UBUS_SWITCHER_SHOWN "SWITCHER_SHOWN"
198+#define UBUS_SWITCHER_START "SWITCHER_SHOWN_START"
199+#define UBUS_SWITCHER_END "SWITCHER_SHOWN_END"
200 #define UBUS_SWITCHER_SELECTION_CHANGED "SWITCHER_SELECTION_CHANGED"
201
202 #endif // UBUS_MESSAGES_H