Unity

Merge lp:~azzar1/unity/fix-1035860 into lp:unity

  1. fix-1035860
  2. Merge into trunk
Proposed by Andrea Azzarone
Status: Merged
Approved by: Martin Mrazik
Approved revision: no longer in the source branch.
Merged at revision: 2874
Proposed branch: lp:~azzar1/unity/fix-1035860
Merge into: lp:unity
Diff against target: 387 lines (+219/-23)
11 files modified
launcher/Decaymulator.h (+1/-1)
launcher/Launcher.cpp (+4/-4)
launcher/LauncherHideMachine.cpp (+5/-3)
launcher/LauncherHideMachine.h (+2/-2)
launcher/LauncherHoverMachine.cpp (+5/-7)
launcher/LauncherHoverMachine.h (+4/-5)
tests/CMakeLists.txt (+2/-0)
tests/autopilot/unity/tests/launcher/test_shortcut.py (+6/-0)
tests/test_launcher.cpp (+0/-1)
tests/test_launcher_hide_machine.cpp (+75/-0)
tests/test_launcher_hover_machine.cpp (+115/-0)
To merge this branch: bzr merge lp:~azzar1/unity/fix-1035860
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Thomi Richards (community) quality Approve
Marco Trevisan (Treviño) Approve
Review via email: mp+129949@code.launchpad.net

Commit message

Move SHORTCUT_KEYS_VISIBLE from hover machine to hide machine.

Description of the change

== Problem ==
[regression] Launcher tooltip should not appear when keyboard overlay is shown.

== Fix ==
Move SHORTCUT_KEYS_VISIBLE from hover machine to hide machine.

== Test ==
* AP test for the bug 1035860.
* unit tests for LauncherHoverMachine.
* unit tests for LauncherHideMachine (just for the hide mode NEVER, will complete the job in another branch later ... :)

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

LGTM

review: Approve
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) :
review: Approve (quality)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/82/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/82/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/82/console

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/83/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/83/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/83/console

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/84/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/84/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/84/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Mrazik (mrazik) wrote :

Jenkins failure -- re-approving.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/Decaymulator.h'
2--- launcher/Decaymulator.h 2012-07-17 16:54:27 +0000
3+++ launcher/Decaymulator.h 2012-10-31 09:34:22 +0000
4@@ -20,7 +20,7 @@
5 #ifndef UNITY_DECAYMULATOR_H
6 #define UNITY_DECAYMULATOR_H
7
8-#include <Nux/Nux.h>
9+#include <NuxCore/Property.h>
10 #include <UnityCore/GLibSource.h>
11
12 namespace unity
13
14=== modified file 'launcher/Launcher.cpp'
15--- launcher/Launcher.cpp 2012-10-30 13:46:32 +0000
16+++ launcher/Launcher.cpp 2012-10-31 09:34:22 +0000
17@@ -288,7 +288,8 @@
18 .add("hide-quirks", _hide_machine.DebugHideQuirks())
19 .add("hover-quirks", _hover_machine.DebugHoverQuirks())
20 .add("icon-size", _icon_size)
21- .add("shortcuts_shown", _shortcuts_shown);
22+ .add("shortcuts_shown", _shortcuts_shown)
23+ .add("tooltip-shown", _active_tooltip != nullptr);
24 }
25
26 void Launcher::SetMousePosition(int x, int y)
27@@ -1157,7 +1158,7 @@
28 void Launcher::ShowShortcuts(bool show)
29 {
30 _shortcuts_shown = show;
31- _hover_machine.SetQuirk(LauncherHoverMachine::SHORTCUT_KEYS_VISIBLE, show);
32+ _hide_machine.SetQuirk(LauncherHideMachine::SHORTCUT_KEYS_VISIBLE, show);
33 EnsureAnimation();
34 }
35
36@@ -1504,7 +1505,6 @@
37
38 void Launcher::SetHover(bool hovered)
39 {
40-
41 if (hovered == _hovered)
42 return;
43
44@@ -1522,7 +1522,7 @@
45
46 if (IsOverlayOpen() && !_hide_machine.GetQuirk(LauncherHideMachine::EXTERNAL_DND_ACTIVE))
47 {
48- if (hovered && !_hover_machine.GetQuirk(LauncherHoverMachine::SHORTCUT_KEYS_VISIBLE))
49+ if (hovered && !_hide_machine.GetQuirk(LauncherHideMachine::SHORTCUT_KEYS_VISIBLE))
50 SaturateIcons();
51 else
52 DesaturateIcons();
53
54=== modified file 'launcher/LauncherHideMachine.cpp'
55--- launcher/LauncherHideMachine.cpp 2012-10-29 09:34:54 +0000
56+++ launcher/LauncherHideMachine.cpp 2012-10-31 09:34:22 +0000
57@@ -97,14 +97,16 @@
58 SCALE_ACTIVE = 1 << 11, 2048 #VISIBLE_REQUIRED
59 EXPO_ACTIVE = 1 << 12, 4096 #VISIBLE_REQUIRED
60 MT_DRAG_OUT = 1 << 13, 8192 #VISIBLE_REQUIRED
61- LAUNCHER_PULSE = 1 << 14, 16384 #VISIBLE_REQUIRED
62- LOCK_HIDE = 1 << 15, 32768
63+ REVEAL_PRESSURE_PASS = 1 << 14, 16384
64+ LAUNCHER_PULSE = 1 << 15, 32768 #VISIBLE_REQUIRED
65+ LOCK_HIDE = 1 << 16, 65536
66+ SHORTCUT_KEYS_VISIBLE = 1 << 17, 131072 #VISIBLE REQUIRED
67 */
68
69 #define VISIBLE_REQUIRED (QUICKLIST_OPEN | EXTERNAL_DND_ACTIVE | \
70 INTERNAL_DND_ACTIVE | TRIGGER_BUTTON_SHOW | VERTICAL_SLIDE_ACTIVE |\
71 KEY_NAV_ACTIVE | PLACES_VISIBLE | SCALE_ACTIVE | EXPO_ACTIVE |\
72-MT_DRAG_OUT | LAUNCHER_PULSE)
73+MT_DRAG_OUT | LAUNCHER_PULSE | SHORTCUT_KEYS_VISIBLE)
74
75 void LauncherHideMachine::EnsureHideState(bool skip_delay)
76 {
77
78=== modified file 'launcher/LauncherHideMachine.h'
79--- launcher/LauncherHideMachine.h 2012-08-10 17:57:50 +0000
80+++ launcher/LauncherHideMachine.h 2012-10-31 09:34:22 +0000
81@@ -21,7 +21,6 @@
82 #define LAUNCHERHIDEMACHINE
83
84 #include <sigc++/sigc++.h>
85-#include <glib.h>
86 #include <string>
87 #include <UnityCore/GLibSource.h>
88
89@@ -62,7 +61,8 @@
90 MT_DRAG_OUT = 1 << 13,
91 REVEAL_PRESSURE_PASS = 1 << 14,
92 LAUNCHER_PULSE = 1 << 15,
93- LOCK_HIDE = 1 << 16
94+ LOCK_HIDE = 1 << 16,
95+ SHORTCUT_KEYS_VISIBLE = 1 << 17
96 } HideQuirk;
97
98 nux::Property<int> reveal_pressure;
99
100=== modified file 'launcher/LauncherHoverMachine.cpp'
101--- launcher/LauncherHoverMachine.cpp 2012-07-18 14:04:21 +0000
102+++ launcher/LauncherHoverMachine.cpp 2012-10-31 09:34:22 +0000
103@@ -33,11 +33,10 @@
104 LAUNCHER_HIDDEN = 1 << 0, 1
105 MOUSE_OVER_LAUNCHER = 1 << 1, 2
106 MOUSE_OVER_BFB = 1 << 2, 4
107- SHORTCUT_KEYS_VISIBLE = 1 << 3, 8
108- QUICKLIST_OPEN = 1 << 4, 16
109- KEY_NAV_ACTIVE = 1 << 5, 32
110- LAUNCHER_IN_ACTION = 1 << 6, 64
111- PLACES_VISIBLE = 1 << 7, 128
112+ QUICKLIST_OPEN = 1 << 3, 8
113+ KEY_NAV_ACTIVE = 1 << 4, 16
114+ LAUNCHER_IN_ACTION = 1 << 5, 32
115+ PLACES_VISIBLE = 1 << 6, 64
116 */
117
118 void
119@@ -52,8 +51,7 @@
120 }
121
122 if (GetQuirk((HoverQuirk)(MOUSE_OVER_LAUNCHER | MOUSE_OVER_BFB |
123- SHORTCUT_KEYS_VISIBLE | KEY_NAV_ACTIVE |
124- QUICKLIST_OPEN | LAUNCHER_IN_ACTION)))
125+ KEY_NAV_ACTIVE | QUICKLIST_OPEN | LAUNCHER_IN_ACTION)))
126 should_hover = true;
127 else
128 should_hover = false;
129
130=== modified file 'launcher/LauncherHoverMachine.h'
131--- launcher/LauncherHoverMachine.h 2012-06-18 02:57:23 +0000
132+++ launcher/LauncherHoverMachine.h 2012-10-31 09:34:22 +0000
133@@ -35,11 +35,10 @@
134 LAUNCHER_HIDDEN = 1 << 0,
135 MOUSE_OVER_LAUNCHER = 1 << 1,
136 MOUSE_OVER_BFB = 1 << 2,
137- SHORTCUT_KEYS_VISIBLE = 1 << 3,
138- QUICKLIST_OPEN = 1 << 4,
139- KEY_NAV_ACTIVE = 1 << 5,
140- LAUNCHER_IN_ACTION = 1 << 6,
141- PLACES_VISIBLE = 1 << 7,
142+ QUICKLIST_OPEN = 1 << 3,
143+ KEY_NAV_ACTIVE = 1 << 4,
144+ LAUNCHER_IN_ACTION = 1 << 5,
145+ PLACES_VISIBLE = 1 << 6,
146 } HoverQuirk;
147
148 LauncherHoverMachine();
149
150=== modified file 'tests/CMakeLists.txt'
151--- tests/CMakeLists.txt 2012-10-22 09:31:20 +0000
152+++ tests/CMakeLists.txt 2012-10-31 09:34:22 +0000
153@@ -197,6 +197,8 @@
154 test_im_text_entry.cpp
155 test_launcher_controller.cpp
156 test_launcher_drag_window.cpp
157+ test_launcher_hide_machine.cpp
158+ test_launcher_hover_machine.cpp
159 test_launcher_icon.cpp
160 test_keyboard_util.cpp
161 test_panel_style.cpp
162
163=== modified file 'tests/autopilot/unity/tests/launcher/test_shortcut.py'
164--- tests/autopilot/unity/tests/launcher/test_shortcut.py 2012-06-21 23:20:18 +0000
165+++ tests/autopilot/unity/tests/launcher/test_shortcut.py 2012-10-31 09:34:22 +0000
166@@ -53,3 +53,9 @@
167 self.addCleanup(self.launcher_instance.switcher_cancel)
168 self.launcher_instance.switcher_prev()
169 self.assertThat(self.launcher_instance.shortcuts_shown, Eventually(Equals(True)))
170+
171+ def test_tooltip_not_shown(self):
172+ """Tooltip must not be shown after revealing the launcher with keyboard
173+ and mouse is not on the launcher.
174+ """
175+ self.assertThat(self.launcher_instance.tooltip_shown, Eventually(Equals(False)))
176
177=== modified file 'tests/test_launcher.cpp'
178--- tests/test_launcher.cpp 2012-10-11 01:44:15 +0000
179+++ tests/test_launcher.cpp 2012-10-31 09:34:22 +0000
180@@ -524,7 +524,6 @@
181 EXPECT_TRUE(add_request);
182 }
183
184-
185 }
186 }
187
188
189=== added file 'tests/test_launcher_hide_machine.cpp'
190--- tests/test_launcher_hide_machine.cpp 1970-01-01 00:00:00 +0000
191+++ tests/test_launcher_hide_machine.cpp 2012-10-31 09:34:22 +0000
192@@ -0,0 +1,75 @@
193+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
194+/*
195+ * Copyright (C) 2012 Canonical Ltd
196+ *
197+ * This program is free software: you can redistribute it and/or modify
198+ * it under the terms of the GNU General Public License version 3 as
199+ * published by the Free Software Foundation.
200+ *
201+ * This program is distributed in the hope that it will be useful,
202+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
203+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
204+ * GNU General Public License for more details.
205+ *
206+ * You should have received a copy of the GNU General Public License
207+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
208+ *
209+ * Authored by: Andrea Azzarone <andrea.azzarone@canonical.com>
210+ */
211+
212+#include <gtest/gtest.h>
213+using namespace testing;
214+
215+#include "launcher/LauncherHideMachine.h"
216+#include "test_utils.h"
217+namespace ul = unity::launcher;
218+
219+namespace {
220+
221+ul::LauncherHideMachine::HideQuirk QUIRKS [] {
222+ ul::LauncherHideMachine::QUICKLIST_OPEN,
223+ ul::LauncherHideMachine::EXTERNAL_DND_ACTIVE,
224+ ul::LauncherHideMachine::INTERNAL_DND_ACTIVE,
225+ ul::LauncherHideMachine::TRIGGER_BUTTON_SHOW,
226+ ul::LauncherHideMachine::DND_PUSHED_OFF,
227+ ul::LauncherHideMachine::MOUSE_MOVE_POST_REVEAL,
228+ ul::LauncherHideMachine::VERTICAL_SLIDE_ACTIVE,
229+ ul::LauncherHideMachine::KEY_NAV_ACTIVE,
230+ ul::LauncherHideMachine::PLACES_VISIBLE,
231+ ul::LauncherHideMachine::SCALE_ACTIVE,
232+ ul::LauncherHideMachine::EXPO_ACTIVE,
233+ ul::LauncherHideMachine::MT_DRAG_OUT,
234+ ul::LauncherHideMachine::REVEAL_PRESSURE_PASS,
235+ ul::LauncherHideMachine::LAUNCHER_PULSE,
236+ ul::LauncherHideMachine::LOCK_HIDE,
237+ ul::LauncherHideMachine::SHORTCUT_KEYS_VISIBLE };
238+
239+struct HideModeNever : public TestWithParam<std::tr1::tuple<ul::LauncherHideMachine::HideQuirk, bool, bool>> {
240+ ul::LauncherHideMachine machine;
241+};
242+
243+TEST_P(HideModeNever, Bool2Bool) {
244+ auto quirk = std::tr1::get<0>(GetParam());
245+ bool initial_value = std::tr1::get<1>(GetParam());
246+ bool final_value = std::tr1::get<2>(GetParam());
247+
248+ machine.SetMode(ul::LauncherHideMachine::HIDE_NEVER);
249+ machine.SetQuirk(quirk, initial_value);
250+
251+ bool sig_received = false;
252+ machine.should_hide_changed.connect([&sig_received] (bool /*value*/) {
253+ sig_received = true;
254+ });
255+
256+ machine.SetQuirk(quirk, final_value);
257+
258+ auto check_function = [&sig_received]() { return sig_received; };
259+ Utils::WaitUntil(check_function, false, 20/1000);
260+}
261+
262+INSTANTIATE_TEST_CASE_P(TestLauncherHideMachine, HideModeNever,
263+ Combine(ValuesIn(QUIRKS), Bool(), Bool()));
264+
265+// TODO: write tests for HideModeAutohide.
266+
267+}
268
269=== added file 'tests/test_launcher_hover_machine.cpp'
270--- tests/test_launcher_hover_machine.cpp 1970-01-01 00:00:00 +0000
271+++ tests/test_launcher_hover_machine.cpp 2012-10-31 09:34:22 +0000
272@@ -0,0 +1,115 @@
273+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
274+/*
275+ * Copyright (C) 2012 Canonical Ltd
276+ *
277+ * This program is free software: you can redistribute it and/or modify
278+ * it under the terms of the GNU General Public License version 3 as
279+ * published by the Free Software Foundation.
280+ *
281+ * This program is distributed in the hope that it will be useful,
282+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
283+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
284+ * GNU General Public License for more details.
285+ *
286+ * You should have received a copy of the GNU General Public License
287+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
288+ *
289+ * Authored by: Andrea Azzarone <andrea.azzarone@canonical.com>
290+ */
291+
292+#include <gtest/gtest.h>
293+using namespace testing;
294+
295+#include "launcher/LauncherHoverMachine.h"
296+#include "test_utils.h"
297+
298+namespace {
299+
300+unity::LauncherHoverMachine::HoverQuirk QUIRKS [] {
301+ unity::LauncherHoverMachine::MOUSE_OVER_LAUNCHER,
302+ unity::LauncherHoverMachine::MOUSE_OVER_BFB,
303+ unity::LauncherHoverMachine::QUICKLIST_OPEN,
304+ unity::LauncherHoverMachine::KEY_NAV_ACTIVE,
305+ unity::LauncherHoverMachine::LAUNCHER_IN_ACTION };
306+
307+struct SingleQuirk : public TestWithParam<std::tr1::tuple<unity::LauncherHoverMachine::HoverQuirk, bool, bool>> {
308+ unity::LauncherHoverMachine machine;
309+};
310+
311+TEST_P(SingleQuirk, Bool2Bool) {
312+ auto quirk = std::tr1::get<0>(GetParam());
313+ bool initial_value = std::tr1::get<1>(GetParam());
314+ bool final_value = std::tr1::get<2>(GetParam());
315+
316+ machine.SetQuirk(quirk, initial_value);
317+ Utils::WaitForTimeoutMSec(20); // ignore the first signal
318+
319+ bool sig_received = false;
320+ bool hover = initial_value;
321+ machine.should_hover_changed.connect([&sig_received, &hover] (bool value) {
322+ sig_received = true;
323+ hover = value;
324+ });
325+
326+ machine.SetQuirk(unity::LauncherHoverMachine::LAUNCHER_HIDDEN, false);
327+ machine.SetQuirk(quirk, final_value);
328+
329+ if (initial_value != final_value)
330+ {
331+ Utils::WaitUntil(sig_received);
332+ ASSERT_EQ(hover, final_value);
333+ }
334+ else
335+ {
336+ Utils::WaitForTimeoutMSec(20);
337+ ASSERT_FALSE(sig_received);
338+ }
339+}
340+
341+INSTANTIATE_TEST_CASE_P(TestLauncherHoverMachine, SingleQuirk,
342+ Combine(ValuesIn(QUIRKS), Bool(), Bool()));
343+
344+
345+struct MultipleQuirks : public TestWithParam<std::tr1::tuple<unity::LauncherHoverMachine::HoverQuirk, bool, bool,
346+ unity::LauncherHoverMachine::HoverQuirk, bool, bool>> {
347+ unity::LauncherHoverMachine machine;
348+};
349+
350+TEST_P(MultipleQuirks, DoubleBool2Bool) {
351+ auto quirk1 = std::tr1::get<0>(GetParam());
352+ auto quirk2 = std::tr1::get<3>(GetParam());
353+
354+ if (quirk1 == quirk2)
355+ return;
356+
357+ bool initial_value1 = std::tr1::get<1>(GetParam());
358+ bool final_value1 = std::tr1::get<2>(GetParam());
359+ bool initial_value2 = std::tr1::get<4>(GetParam());
360+ bool final_value2 = std::tr1::get<5>(GetParam());
361+
362+ machine.SetQuirk(quirk1, initial_value1);
363+ machine.SetQuirk(quirk2, initial_value2);
364+ Utils::WaitForTimeoutMSec(20);
365+
366+ bool sig_received = false;
367+ bool hover = initial_value1 || initial_value2;
368+ machine.should_hover_changed.connect([&sig_received, &hover] (bool value) {
369+ sig_received = true;
370+ hover = value;
371+ });
372+
373+ machine.SetQuirk(unity::LauncherHoverMachine::LAUNCHER_HIDDEN, false);
374+ machine.SetQuirk(quirk1, final_value1);
375+ machine.SetQuirk(quirk2, final_value2);
376+
377+ if ((initial_value1 || initial_value2) != (final_value1 || final_value2))
378+ Utils::WaitUntil(sig_received);
379+
380+ auto check_function = [&]() { return hover == (final_value1 || final_value2); };
381+ Utils::WaitUntil(check_function, true, 20/1000);
382+}
383+
384+INSTANTIATE_TEST_CASE_P(TestLauncherHoverMachine, MultipleQuirks,
385+ Combine(ValuesIn(QUIRKS), Bool(), Bool(), ValuesIn(QUIRKS), Bool(), Bool()));
386+
387+}