Merge lp:~3v1n0/unity/alt+tab-scroll-wheel into lp:unity

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 2090
Proposed branch: lp:~3v1n0/unity/alt+tab-scroll-wheel
Merge into: lp:unity
Diff against target: 277 lines (+137/-13)
4 files modified
plugins/unityshell/src/unityshell.cpp (+46/-8)
plugins/unityshell/src/unityshell.h (+2/-1)
tests/autopilot/autopilot/emulators/unity/switcher.py (+16/-3)
tests/autopilot/autopilot/tests/test_switcher.py (+73/-1)
To merge this branch: bzr merge lp:~3v1n0/unity/alt+tab-scroll-wheel
Reviewer Review Type Date Requested Status
Alex Launi (community) Approve
Thomi Richards (community) Approve
Review via email: mp+94675@code.launchpad.net

Description of the change

Partly fixes bug #824965 enabling scroll wheel on Alt+Tab to switch the active application.

To post a comment you must log in.
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Looks good, but with a few things that need fixing.

1) The docstrings for next_icon_mouse and previous_icon_mouse should say: "icon" instead of "application".

2) If possible, we should use constsants to define the mouse buttons in unity. These must be defined in X somewhere?

3)Your tests need to be split up, like this:

107 + def test_switcher_scroll_next(self):
108 + self.set_timeout_setting(False)
109 + sleep(1)
110 +
111 + self.switcher.initiate()
112 + sleep(.2)
113 +
114 + start = self.switcher.get_selection_index()
115 + self.switcher.next_icon_mouse()
116 + sleep(.2)
117 +
118 + end = self.switcher.get_selection_index()
119 + self.assertThat(start, NotEquals(0))
120 + self.assertThat(end, Equals(start + 1))
121 +

def test_switcher_ignores_fast_wheel_down_events(self):
122 + # Quickly repeatead events should be ignored (except the first)
123 + start = self.switcher.get_selection_index()
124 + self.switcher.next_icon_mouse()
125 + self.switcher.next_icon_mouse()
126 + sleep(.2)
127 +
128 + end = self.switcher.get_selection_index()
129 + self.assertThat(end, Equals(start + 1))
130 +
131 + self.switcher.terminate()
132 +

133 + def test_switcher_scroll_prev(self):
134 + self.set_timeout_setting(False)
135 + sleep(1)
136 +
137 + self.switcher.initiate()
138 + sleep(.2)
139 +
140 + start = self.switcher.get_selection_index()
141 + self.switcher.previous_icon_mouse()
142 + sleep(.2)
143 +
144 + end = self.switcher.get_selection_index()
145 + self.assertThat(start, NotEquals(0))
146 + self.assertThat(end, Equals(start - 1))
147 +

def test_switcher_ignores_fast_wheel_down_events(self):
148 + # Quickly repeatead events should be ignored (except the first)
149 + start = self.switcher.get_selection_index()
150 + self.switcher.previous_icon_mouse()
151 + self.switcher.previous_icon_mouse()
152 + sleep(.2)
153 +
154 + end = self.switcher.get_selection_index()
155 + self.assertThat(end, Equals(start - 1))
156 +
157 + self.switcher.terminate()
158 + self.set_timeout_setting(True)
159 +

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

Looks good

review: Approve
Revision history for this message
Alex Launi (alexlauni) wrote :

This currently has conflicts with trunk in plugins/unityshell/src/unityshell.cpp

review: Needs Fixing
Revision history for this message
Alex Launi (alexlauni) wrote :

nice

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/431/console reported an error when processing this lp:~3v1n0/unity/alt+tab-scroll-wheel branch.
Not merging it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp 2012-03-06 18:24:09 +0000
+++ plugins/unityshell/src/unityshell.cpp 2012-03-07 17:27:25 +0000
@@ -93,6 +93,8 @@
93{93{
94// Tap duration in milliseconds.94// Tap duration in milliseconds.
95const int ALT_TAP_DURATION = 250;95const int ALT_TAP_DURATION = 250;
96const unsigned int SCROLL_DOWN_BUTTON = 6;
97const unsigned int SCROLL_UP_BUTTON = 7;
96} // namespace local98} // namespace local
97} // anon namespace99} // anon namespace
98100
@@ -124,6 +126,7 @@
124 , dash_is_open_ (false)126 , dash_is_open_ (false)
125 , grab_index_ (0)127 , grab_index_ (0)
126 , painting_tray_ (false)128 , painting_tray_ (false)
129 , last_scroll_event_(0)
127 , last_hud_show_time_(0)130 , last_hud_show_time_(0)
128{131{
129 Timer timer;132 Timer timer;
@@ -1340,6 +1343,25 @@
1340 EnableCancelAction(false);1343 EnableCancelAction(false);
1341 }1344 }
1342 break;1345 break;
1346 case ButtonRelease:
1347 if (switcher_controller_ && switcher_controller_->Visible())
1348 {
1349 XButtonEvent *bev = reinterpret_cast<XButtonEvent*>(event);
1350 if (bev->time - last_scroll_event_ > 150)
1351 {
1352 if (bev->button == Button4 || bev->button == local::SCROLL_UP_BUTTON)
1353 {
1354 switcher_controller_->Prev();
1355 last_scroll_event_ = bev->time;
1356 }
1357 else if (bev->button == Button5 || bev->button == local::SCROLL_DOWN_BUTTON)
1358 {
1359 switcher_controller_->Next();
1360 last_scroll_event_ = bev->time;
1361 }
1362 }
1363 }
1364 break;
1343 case KeyPress:1365 case KeyPress:
1344 {1366 {
1345 KeySym key_sym;1367 KeySym key_sym;
@@ -1601,7 +1623,7 @@
1601 return true;1623 return true;
1602}1624}
16031625
1604bool UnityScreen::altTabInitiateCommon(switcher::ShowMode show_mode)1626bool UnityScreen::altTabInitiateCommon(CompAction* action, switcher::ShowMode show_mode)
1605{1627{
1606 if (!grab_index_)1628 if (!grab_index_)
1607 grab_index_ = screen->pushGrab (screen->invisibleCursor(), "unity-switcher");1629 grab_index_ = screen->pushGrab (screen->invisibleCursor(), "unity-switcher");
@@ -1619,6 +1641,14 @@
1619 screen->addAction(&optionGetAltTabDetailStop());1641 screen->addAction(&optionGetAltTabDetailStop());
1620 screen->addAction(&optionGetAltTabLeft());1642 screen->addAction(&optionGetAltTabLeft());
16211643
1644 /* Create a new keybinding for scroll buttons and current modifiers */
1645 CompAction scroll_up;
1646 CompAction scroll_down;
1647 scroll_up.setButton(CompAction::ButtonBinding(local::SCROLL_UP_BUTTON, action->key().modifiers()));
1648 scroll_down.setButton(CompAction::ButtonBinding(local::SCROLL_DOWN_BUTTON, action->key().modifiers()));
1649 screen->addAction(&scroll_up);
1650 screen->addAction(&scroll_down);
1651
1622 // maybe check launcher position/hide state?1652 // maybe check launcher position/hide state?
16231653
1624 int device = screen->outputDeviceForPoint (pointerX, pointerY);1654 int device = screen->outputDeviceForPoint (pointerX, pointerY);
@@ -1655,10 +1685,18 @@
1655 screen->removeGrab(grab_index_, NULL);1685 screen->removeGrab(grab_index_, NULL);
1656 grab_index_ = 0;1686 grab_index_ = 0;
16571687
1658 screen->removeAction (&optionGetAltTabRight ());1688 screen->removeAction(&optionGetAltTabRight ());
1659 screen->removeAction (&optionGetAltTabDetailStart ());1689 screen->removeAction(&optionGetAltTabDetailStart ());
1660 screen->removeAction (&optionGetAltTabDetailStop ());1690 screen->removeAction(&optionGetAltTabDetailStop ());
1661 screen->removeAction (&optionGetAltTabLeft ());1691 screen->removeAction(&optionGetAltTabLeft ());
1692
1693 /* Removing the scroll actions */
1694 CompAction scroll_up;
1695 CompAction scroll_down;
1696 scroll_up.setButton(CompAction::ButtonBinding(local::SCROLL_UP_BUTTON, action->key().modifiers()));
1697 scroll_down.setButton(CompAction::ButtonBinding(local::SCROLL_DOWN_BUTTON, action->key().modifiers()));
1698 screen->removeAction(&scroll_up);
1699 screen->removeAction(&scroll_down);
16621700
1663 bool accept_state = (state & CompAction::StateCancel) == 0;1701 bool accept_state = (state & CompAction::StateCancel) == 0;
1664 switcher_controller_->Hide(accept_state);1702 switcher_controller_->Hide(accept_state);
@@ -1675,7 +1713,7 @@
1675 if (switcher_controller_->Visible())1713 if (switcher_controller_->Visible())
1676 switcher_controller_->Next();1714 switcher_controller_->Next();
1677 else1715 else
1678 altTabInitiateCommon(switcher::ShowMode::CURRENT_VIEWPORT);1716 altTabInitiateCommon(action, switcher::ShowMode::CURRENT_VIEWPORT);
16791717
1680 action->setState(action->state() | CompAction::StateTermKey);1718 action->setState(action->state() | CompAction::StateTermKey);
1681 return true;1719 return true;
@@ -1688,7 +1726,7 @@
1688 if (switcher_controller_->Visible())1726 if (switcher_controller_->Visible())
1689 switcher_controller_->Next();1727 switcher_controller_->Next();
1690 else1728 else
1691 altTabInitiateCommon(switcher::ShowMode::ALL);1729 altTabInitiateCommon(action, switcher::ShowMode::ALL);
16921730
1693 action->setState(action->state() | CompAction::StateTermKey);1731 action->setState(action->state() | CompAction::StateTermKey);
1694 return true;1732 return true;
@@ -1730,7 +1768,7 @@
1730{1768{
1731 if (!switcher_controller_->Visible())1769 if (!switcher_controller_->Visible())
1732 {1770 {
1733 altTabInitiateCommon(switcher::ShowMode::CURRENT_VIEWPORT);1771 altTabInitiateCommon(action, switcher::ShowMode::CURRENT_VIEWPORT);
1734 switcher_controller_->Select(1); // always select the current application1772 switcher_controller_->Select(1); // always select the current application
1735 }1773 }
17361774
17371775
=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h 2012-03-06 18:24:09 +0000
+++ plugins/unityshell/src/unityshell.h 2012-03-07 17:27:25 +0000
@@ -192,7 +192,7 @@
192 bool executeCommand(CompAction* action, CompAction::State state, CompOption::Vector& options);192 bool executeCommand(CompAction* action, CompAction::State state, CompOption::Vector& options);
193 bool setKeyboardFocusKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);193 bool setKeyboardFocusKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
194194
195 bool altTabInitiateCommon(switcher::ShowMode mode);195 bool altTabInitiateCommon(CompAction* action, switcher::ShowMode mode);
196 bool altTabTerminateCommon(CompAction* action,196 bool altTabTerminateCommon(CompAction* action,
197 CompAction::State state,197 CompAction::State state,
198 CompOption::Vector& options);198 CompOption::Vector& options);
@@ -331,6 +331,7 @@
331 CompWindowList fullscreen_windows_;331 CompWindowList fullscreen_windows_;
332 bool painting_tray_;332 bool painting_tray_;
333 unsigned int tray_paint_mask_;333 unsigned int tray_paint_mask_;
334 unsigned int last_scroll_event_;
334 gint64 last_hud_show_time_;335 gint64 last_hud_show_time_;
335336
336 GLMatrix panel_shadow_matrix_;337 GLMatrix panel_shadow_matrix_;
337338
=== modified file 'tests/autopilot/autopilot/emulators/unity/switcher.py'
--- tests/autopilot/autopilot/emulators/unity/switcher.py 2012-03-06 19:03:00 +0000
+++ tests/autopilot/autopilot/emulators/unity/switcher.py 2012-03-07 17:27:25 +0000
@@ -12,7 +12,7 @@
1212
13from autopilot.keybindings import KeybindingsHelper13from autopilot.keybindings import KeybindingsHelper
14from autopilot.emulators.unity import get_state_by_path, make_introspection_object14from autopilot.emulators.unity import get_state_by_path, make_introspection_object
15from autopilot.emulators.X11 import Keyboard15from autopilot.emulators.X11 import Keyboard, Mouse
1616
17# even though we don't use these directly, we need to make sure they've been17# even though we don't use these directly, we need to make sure they've been
18# imported so the classes contained are registered with the introspection API.18# imported so the classes contained are registered with the introspection API.
@@ -29,6 +29,7 @@
29 def __init__(self):29 def __init__(self):
30 super(Switcher, self).__init__()30 super(Switcher, self).__init__()
31 self._keyboard = Keyboard()31 self._keyboard = Keyboard()
32 self._mouse = Mouse()
3233
33 def initiate(self):34 def initiate(self):
34 """Start the switcher with alt+tab."""35 """Start the switcher with alt+tab."""
@@ -78,15 +79,27 @@
78 self.keybinding_release("switcher/reveal_normal")79 self.keybinding_release("switcher/reveal_normal")
7980
80 def next_icon(self):81 def next_icon(self):
81 """Move to the next application."""82 """Move to the next icon."""
82 logger.debug("Selecting next item in switcher.")83 logger.debug("Selecting next item in switcher.")
83 self.keybinding("switcher/next")84 self.keybinding("switcher/next")
8485
85 def previous_icon(self):86 def previous_icon(self):
86 """Move to the previous application."""87 """Move to the previous icon."""
87 logger.debug("Selecting previous item in switcher.")88 logger.debug("Selecting previous item in switcher.")
88 self.keybinding("switcher/prev")89 self.keybinding("switcher/prev")
8990
91 def next_icon_mouse(self):
92 """Move to the next icon using the mouse scroll wheel"""
93 logger.debug("Selecting next item in switcher with mouse scroll wheel.")
94 self._mouse.press(6)
95 self._mouse.release(6)
96
97 def previous_icon_mouse(self):
98 """Move to the previous icon using the mouse scroll wheel"""
99 logger.debug("Selecting previous item in switcher with mouse scroll wheel.")
100 self._mouse.press(7)
101 self._mouse.release(7)
102
90 def show_details(self):103 def show_details(self):
91 """Show detail mode."""104 """Show detail mode."""
92 logger.debug("Showing details view.")105 logger.debug("Showing details view.")
93106
=== modified file 'tests/autopilot/autopilot/tests/test_switcher.py'
--- tests/autopilot/autopilot/tests/test_switcher.py 2012-03-06 19:03:00 +0000
+++ tests/autopilot/autopilot/tests/test_switcher.py 2012-03-07 17:27:25 +0000
@@ -65,7 +65,79 @@
6565
66 self.assertThat(start, NotEquals(0))66 self.assertThat(start, NotEquals(0))
67 self.assertThat(end, Equals(start - 1))67 self.assertThat(end, Equals(start - 1))
68 self.set_timeout_setting(True)68
69 def test_switcher_scroll_next(self):
70 self.set_timeout_setting(False)
71 sleep(1)
72
73 self.switcher.initiate()
74 sleep(.2)
75
76 start = self.switcher.get_selection_index()
77 self.switcher.next_icon_mouse()
78 sleep(.2)
79
80 end = self.switcher.get_selection_index()
81 self.assertThat(start, NotEquals(0))
82 self.assertThat(end, Equals(start + 1))
83
84 self.switcher.terminate()
85
86 def test_switcher_scroll_prev(self):
87 self.set_timeout_setting(False)
88 sleep(1)
89
90 self.switcher.initiate()
91 sleep(.2)
92
93 start = self.switcher.get_selection_index()
94 self.switcher.previous_icon_mouse()
95 sleep(.2)
96
97 end = self.switcher.get_selection_index()
98 self.assertThat(start, NotEquals(0))
99 self.assertThat(end, Equals(start - 1))
100
101 self.switcher.terminate()
102
103 def test_switcher_scroll_next_ignores_fast_events(self):
104 self.set_timeout_setting(False)
105 sleep(1)
106
107 self.switcher.initiate()
108 sleep(.2)
109
110 # Quickly repeatead events should be ignored (except the first)
111 start = self.switcher.get_selection_index()
112 self.switcher.next_icon_mouse()
113 self.switcher.next_icon_mouse()
114 self.switcher.next_icon_mouse()
115 sleep(.2)
116
117 end = self.switcher.get_selection_index()
118 self.assertThat(start, NotEquals(0))
119 self.assertThat(end, Equals(start + 1))
120
121 self.switcher.terminate()
122
123 def test_switcher_scroll_prev_ignores_fast_events(self):
124 self.set_timeout_setting(False)
125 sleep(1)
126
127 self.switcher.initiate()
128 sleep(.2)
129
130 # Quickly repeatead events should be ignored (except the first)
131 start = self.switcher.get_selection_index()
132 self.switcher.previous_icon_mouse()
133 self.switcher.previous_icon_mouse()
134 self.switcher.previous_icon_mouse()
135 sleep(.2)
136
137 end = self.switcher.get_selection_index()
138 self.assertThat(end, Equals(start - 1))
139
140 self.switcher.terminate()
69141
70 def test_switcher_arrow_key_does_not_init(self):142 def test_switcher_arrow_key_does_not_init(self):
71 self.set_timeout_setting(False)143 self.set_timeout_setting(False)