Merge lp:~townsend/unity/fix-lp1286784 into lp:unity

[Christopher Townsend]

Fix the inconsistent z ordering of windows when using the mouse to scroll the Launcher icon of the active application.

A user (asmoore82) described the current behavior of scrolling over the Launcher icon to focus windows as "inconsistent" and he gave a very detailed explanation as to why (see bug #1286784).

I spoke with John Lea about this and he agreed that the scrolling behavior should be more in line with what asmorre82 describes. asmoore82 attached a patch to the bug, so this MP is incorporating that patch plus fixing some things like removing an unused variable and fixing up a test based on the new behavior.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3706
http://jenkins.qa.ubuntu.com/job/unity-ci/793/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/329
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/327

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/793/rebuild

review: Approve (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

Nice. LGTM

review: Approve

=== modified file 'launcher/ApplicationLauncherIcon.cpp'
--- launcher/ApplicationLauncherIcon.cpp	2014-02-13 10:14:20 +0000
+++ launcher/ApplicationLauncherIcon.cpp	2014-03-12 20:07:49 +0000
@@ -68,7 +68,6 @@
   : SimpleLauncherIcon(IconType::APPLICATION)
   , _startup_notification_timestamp(0)
   , _last_scroll_timestamp(0)
-  , _last_scroll_direction(ScrollDirection::DOWN)
   , _progressive_scroll(0)
   , use_custom_bg_color_(false)
   , bg_color_(nux::color::White)
@@ -1276,19 +1275,17 @@
 
 void PerformScrollUp(WindowList const& windows, unsigned int progressive_scroll)
 {
-  if (!progressive_scroll)
-  {
-    windows.at(1)->Focus();
-    return;
-  }
-  else if (progressive_scroll == 1)
-  {
+  if (progressive_scroll == windows.size() - 1)
+  {
+    //RestackAbove to preserve Global Stacking Order
+    WindowManager::Default().RestackBelow(windows.at(0)->window_id(), windows.at(1)->window_id());
+    WindowManager::Default().RestackBelow(windows.at(1)->window_id(), windows.at(0)->window_id());
     windows.back()->Focus();
     return;
   }
 
-  WindowManager::Default().RestackBelow(windows.at(0)->window_id(), windows.at(windows.size() - progressive_scroll + 1)->window_id());
-  windows.at(windows.size() - progressive_scroll + 1)->Focus();
+  WindowManager::Default().RestackBelow(windows.at(0)->window_id(), windows.at(progressive_scroll + 1)->window_id());
+  windows.at(progressive_scroll + 1)->Focus();
 }
 
 void PerformScrollDown(WindowList const& windows, unsigned int progressive_scroll)
@@ -1308,18 +1305,21 @@
 {
   if (timestamp - _last_scroll_timestamp < 150)
     return;
-  else if (timestamp - _last_scroll_timestamp > 1500 || direction != _last_scroll_direction)
+  else if (timestamp - _last_scroll_timestamp > 1500)
     _progressive_scroll = 0;
 
   _last_scroll_timestamp = timestamp;
-  _last_scroll_direction = direction;
 
   auto const& windows = GetWindowsOnCurrentDesktopInStackingOrder();
 
   if (!IsActive() || windows.size() <= 1)
     return;
 
-  ++_progressive_scroll;
+  if (direction == ScrollDirection::DOWN)
+    ++_progressive_scroll;
+  else
+    //--_progressive_scroll; but roll to the top of windows
+    _progressive_scroll += windows.size() - 1;
   _progressive_scroll %= windows.size();
 
   switch(direction)

=== modified file 'launcher/ApplicationLauncherIcon.h'
--- launcher/ApplicationLauncherIcon.h	2013-11-14 00:17:19 +0000
+++ launcher/ApplicationLauncherIcon.h	2014-03-12 20:07:49 +0000
@@ -139,7 +139,6 @@
   std::string _remote_uri;
   Time _startup_notification_timestamp;
   Time _last_scroll_timestamp;
-  ScrollDirection _last_scroll_direction;
   unsigned int _progressive_scroll;
   std::set<std::string> _supported_types;
   std::vector<glib::Object<DbusmenuMenuitem>> _menu_items;

=== modified file 'tests/test_application_launcher_icon.cpp'
--- tests/test_application_launcher_icon.cpp	2014-01-15 14:51:10 +0000
+++ tests/test_application_launcher_icon.cpp	2014-03-12 20:07:49 +0000
@@ -668,8 +668,14 @@
   mock_icon->PerformScroll(AbstractLauncherIcon::ScrollDirection::DOWN, 200);
   EXPECT_THAT(WM->GetWindowsInStackingOrder(), testing::ElementsAre(7, 6, 5, 4, 3, 1, 2));
 
-  mock_icon->PerformScroll(AbstractLauncherIcon::ScrollDirection::UP, 400);
-  EXPECT_THAT(WM->GetWindowsInStackingOrder(), testing::ElementsAre(7, 6, 4, 3, 1, 2, 5));
+  mock_icon->PerformScroll(AbstractLauncherIcon::ScrollDirection::DOWN, 400);
+  EXPECT_THAT(WM->GetWindowsInStackingOrder(), testing::ElementsAre(7, 6, 5, 4, 2, 1, 3));
+
+  mock_icon->PerformScroll(AbstractLauncherIcon::ScrollDirection::UP, 600);
+  EXPECT_THAT(WM->GetWindowsInStackingOrder(), testing::ElementsAre(7, 6, 5, 4, 3, 1, 2));
+
+  mock_icon->PerformScroll(AbstractLauncherIcon::ScrollDirection::UP, 800);
+  EXPECT_THAT(WM->GetWindowsInStackingOrder(), testing::ElementsAre(7, 6, 5, 4, 3, 2, 1));
 }
 
 TEST_F(TestApplicationLauncherIcon, PerformScrollNoWindows)