Merge lp:~3v1n0/unity/switcher-noinputwin into lp:unity

[Marco Trevisan (Treviño)]

SwitcherController: use again the SwitcherView as non-input window

Otherwise it could only lead to focusing troubles.
Added utilities to manage the WindowManager default close-window keybinding and using
it in our views.

SwitcherWindow has been made an input window some time ago to fix bug #926406. Also if that's a good workaround, it is not the best approach because it makes the WindowManager to be distracted by the switcher window as well (that should only be an overlay) moving the focus to it. This also needed a workaround to fix #1071298.

So, I disabled again the input window (we'll still be able to handle mouse events in future if needed, thanks to the grab), and to fix for good the Alt+F4 bug (lp:926406) I added few utility functions that saves into a property in WindowManager the default keybinding that should be used to close a view that now is finally dynamic.

Using it to close the Switcher (instead of the window below) and updated the code for HUD and Dash overlays.

Ap test updated. Other changes are covered by existing tests.

[Brandon Schaefer (brandontschaefer)]

Yay, im happy you fixed this problem I introduced. Everything still work, and code looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
Unapproved changes made after approval.
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/380/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=amd64/380/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=i386/380/console

review: Needs Fixing (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

Approved!

review: Approve

=== modified file 'launcher/SwitcherController.cpp'
--- launcher/SwitcherController.cpp	2013-01-25 19:09:28 +0000
+++ launcher/SwitcherController.cpp	2013-02-01 03:24:22 +0000
@@ -250,7 +250,7 @@
 
 void Controller::Impl::Show(ShowMode show, SortMode sort, std::vector<AbstractLauncherIcon::Ptr> results)
 {
-  if (results.empty())
+  if (results.empty() || obj_->visible_)
     return;
 
   if (sort == SortMode::FOCUS_ORDER)
@@ -258,18 +258,13 @@
     std::sort(results.begin(), results.end(), CompareSwitcherItemsPriority);
   }
 
-  model_.reset(new SwitcherModel(results));
+  model_ = std::make_shared<SwitcherModel>(results);
   obj_->AddChild(model_.get());
   model_->selection_changed.connect(sigc::mem_fun(this, &Controller::Impl::OnModelSelectionChanged));
   model_->only_detail_on_viewport = (show == ShowMode::CURRENT_VIEWPORT);
 
   SelectFirstItem();
 
-  // XXX: Workaround for a problem related to Alt+TAB which is needed since the
-  //   switcher is set as the active window (LP: #1071298)
-  if (model_->Selection()->GetQuirk(AbstractLauncherIcon::Quirk::ACTIVE))
-    last_active_selection_ = model_->Selection();
-
   obj_->visible_ = true;
 
   if (timeout_length > 0)
@@ -339,8 +334,6 @@
     view_window_->ShowWindow(true);
     view_window_->PushToFront();
     view_window_->SetOpacity(1.0f);
-    view_window_->EnableInputWindow(true, "Switcher", true /* take focus */, false);
-    view_window_->SetInputFocus();
     view_window_->CaptureMouseDownAnyWhereElse(true);
   }
 }
@@ -359,8 +352,6 @@
     view_window_->SetLayout(main_layout_);
     view_window_->SetBackgroundColor(nux::Color(0x00000000));
     view_window_->SetGeometry(workarea_);
-    view_window_->EnableInputWindow(true, "Switcher", false, false);
-    view_window_->InputWindowEnableStruts(false);
   }
 }
 
@@ -416,13 +407,10 @@
     view_window_->SetOpacity(0.0f);
     view_window_->ShowWindow(false);
     view_window_->PushToBack();
-    view_window_->EnableInputWindow(false);
   }
 
   ubus_manager_.SendMessage(UBUS_SWITCHER_SHOWN, g_variant_new("(bi)", false, obj_->monitor_));
 
-  last_active_selection_ = nullptr;
-
   view_.Release();
 }
 
@@ -563,11 +551,13 @@
     {
       if (model_->detail_selection)
       {
-        window =  model_->DetailSelectionWindow();
+        window = model_->DetailSelectionWindow();
       }
-      else if (application == last_active_selection_ && !model_->DetailXids().empty())
+      else if (application->GetQuirk(AbstractLauncherIcon::Quirk::ACTIVE))
       {
-        window =  model_->DetailXids()[0];
+        auto const& xids = model_->DetailXids();
+        if (!xids.empty())
+          window = xids.front();
       }
     }
   }

=== modified file 'launcher/SwitcherControllerImpl.h'
--- launcher/SwitcherControllerImpl.h	2013-01-25 19:09:28 +0000
+++ launcher/SwitcherControllerImpl.h	2013-02-01 03:24:22 +0000
@@ -92,8 +92,6 @@
   nux::HLayout* main_layout_;
   nux::Color bg_color_;
 
-  launcher::AbstractLauncherIcon::Ptr last_active_selection_;
-
   UBusManager ubus_manager_;
   glib::SourceManager sources_;
 };

=== modified file 'launcher/SwitcherModel.cpp'
--- launcher/SwitcherModel.cpp	2012-12-18 00:09:23 +0000
+++ launcher/SwitcherModel.cpp	2013-02-01 03:24:22 +0000
@@ -32,15 +32,14 @@
 
 
 SwitcherModel::SwitcherModel(std::vector<AbstractLauncherIcon::Ptr> const& icons)
-  : applications_(icons)
+  : detail_selection(false)
+  , detail_selection_index(0)
+  , only_detail_on_viewport(false)
+  , applications_(icons)
   , index_(0)
   , last_index_(0)
 {
-  detail_selection = false;
-  detail_selection_index = 0;
-  only_detail_on_viewport = false;
-
-  for (auto application : applications_)
+  for (auto const& application : applications_)
   {
     AddChild(application.GetPointer());
     if (application->GetQuirk(AbstractLauncherIcon::Quirk::ACTIVE))
@@ -136,31 +135,25 @@
 
 std::vector<Window> SwitcherModel::DetailXids()
 {
+  WindowManager& wm = WindowManager::Default();
   std::vector<Window> results;
+
   for (auto& window : Selection()->Windows())
   {
-    results.push_back(window->window_id());
-  }
+    Window xid = window->window_id();
 
-  WindowManager& wm = WindowManager::Default();
-  if (only_detail_on_viewport)
-  {
-    results.erase(std::remove_if(results.begin(), results.end(),
-        [&wm](Window xid) { return !wm.IsWindowOnCurrentDesktop(xid); }),
-        results.end());
+    if (!only_detail_on_viewport || wm.IsWindowOnCurrentDesktop(xid))
+      results.push_back(xid);
   }
 
   std::sort(results.begin(), results.end(), [&wm](Window first, Window second) {
       return wm.GetWindowActiveNumber(first) > wm.GetWindowActiveNumber(second);
   });
 
-
-  if (Selection() == last_active_application_ && results.size () > 1)
+  if (Selection() == last_active_application_ && results.size() > 1)
   {
-    for (unsigned int i = 0; i < results.size()-1; i++)
-    {
-      std::swap (results[i], results[i+1]);
-    }
+    results.push_back(results.front());
+    results.erase(results.begin());
   }
 
   return results;

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2013-01-30 02:11:19 +0000
+++ plugins/unityshell/src/unityshell.cpp	2013-02-01 03:24:22 +0000
@@ -877,7 +877,7 @@
   {
     /* Create a new keybinding for the Escape key and the current modifiers,
      * compiz will take of the ref-counting of the repeated actions */
-    KeyCode escape = XKeysymToKeycode(screen->dpy(), XStringToKeysym("Escape"));
+    KeyCode escape = XKeysymToKeycode(screen->dpy(), XK_Escape);
     CompAction::KeyBinding binding(escape, modifiers);
 
     CompActionPtr &escape_action = _escape_actions[target];
@@ -1623,6 +1623,16 @@
           break;
         }
       }
+      else if (switcher_controller_->Visible())
+      {
+        auto const& close_key = wm.close_window_key();
+
+        if (key_sym == close_key.second && XModifiersToNux(event->xkey.state) == close_key.first)
+        {
+          switcher_controller_->Hide(false);
+          skip_other_plugins = true;
+        }
+      }
 
       if (result > 0)
       {
@@ -1693,10 +1703,9 @@
   }
 
   if (!skip_other_plugins &&
-      screen->otherGrabExist("deco", "move", "switcher", "resize", NULL) &&
-      !switcher_controller_->Visible())
+      screen->otherGrabExist("deco", "move", "switcher", "resize", "unity-switcher", nullptr))
   {
-    wt->ProcessForeignEvent(event, NULL);
+    wt->ProcessForeignEvent(event, nullptr);
   }
 }
 
@@ -2135,37 +2144,28 @@
 
 void UnityScreen::OnLauncherEndKeyNav(GVariant* data)
 {
-  RestoreWindow(data);
+  // Return input-focus to previously focused window (before key-nav-mode was
+  // entered)
+  if (data && g_variant_get_boolean(data))
+    PluginAdapter::Default().RestoreInputFocus();
 }
 
 void UnityScreen::OnSwitcherStart(GVariant* data)
 {
   if (switcher_controller_->Visible())
   {
-    SaveInputThenFocus(switcher_controller_->GetSwitcherInputWindowId());
     UnityWindow::SetupSharedTextures();
   }
 }
 
 void UnityScreen::OnSwitcherEnd(GVariant* data)
 {
-  RestoreWindow(data);
   UnityWindow::CleanupSharedTextures();
 
   for (UnityWindow* uwin : fake_decorated_windows_)
     uwin->CleanupCachedTextures();
 }
 
-void UnityScreen::RestoreWindow(GVariant* data)
-{
-  bool preserve_focus = data ? g_variant_get_boolean(data) : false;
-
-  // Return input-focus to previously focused window (before key-nav-mode was
-  // entered)
-  if (preserve_focus)
-    PluginAdapter::Default().RestoreInputFocus();
-}
-
 bool UnityScreen::SaveInputThenFocus(const guint xid)
 {
   // get CompWindow*
@@ -2270,10 +2270,74 @@
   return ShowHud();
 }
 
+unsigned UnityScreen::CompizModifiersToNux(unsigned input) const
+{
+  unsigned modifiers = 0;
+
+  if (input & CompAltMask)
+  {
+    input &= ~CompAltMask;
+    input |= Mod1Mask;
+  }
+
+  if (modifiers & CompSuperMask)
+  {
+    input &= ~CompSuperMask;
+    input |= Mod4Mask;
+  }
+
+  return XModifiersToNux(input);
+}
+
+unsigned UnityScreen::XModifiersToNux(unsigned input) const
+{
+  unsigned modifiers = 0;
+
+  if (input & Mod1Mask)
+    modifiers |= nux::KEY_MODIFIER_ALT;
+
+  if (input & ShiftMask)
+    modifiers |= nux::KEY_MODIFIER_SHIFT;
+
+  if (input & ControlMask)
+    modifiers |= nux::KEY_MODIFIER_CTRL;
+
+  if (input & Mod4Mask)
+    modifiers |= nux::KEY_MODIFIER_SUPER;
+
+  return modifiers;
+}
+
+void UnityScreen::UpdateCloseWindowKey(CompAction::KeyBinding const& keybind)
+{
+  KeySym keysym = XkbKeycodeToKeysym(screen->dpy(), keybind.keycode(), 0, 0);
+  unsigned modifiers = CompizModifiersToNux(keybind.modifiers());
+
+  WindowManager::Default().close_window_key = std::make_pair(modifiers, keysym);
+}
+
 bool UnityScreen::initPluginActions()
 {
-  CompPlugin* p = CompPlugin::find("expo");
   PluginAdapter& adapter = PluginAdapter::Default();
+
+  CompPlugin* p = CompPlugin::find("core");
+
+  if (p)
+  {
+    MultiActionList expoActions;
+
+    for (CompOption& option : p->vTable->getOptions())
+    {
+      if (option.name() == "close_window_key")
+      {
+        UpdateCloseWindowKey(option.value().action().key());
+        break;
+      }
+    }
+  }
+
+  p = CompPlugin::find("expo");
+
   if (p)
   {
     MultiActionList expoActions;
@@ -2477,7 +2541,8 @@
     }
   }
 
-  if (WindowManager::Default().IsScaleActive() && ScaleScreen::get(screen)->getSelectedWindow() == window->id())
+  if (WindowManager::Default().IsScaleActive() &&
+      ScaleScreen::get(screen)->getSelectedWindow() == window->id())
   {
     nux::Geometry const& scaled_geo = GetScaledGeometry();
     paintInnerGlow(scaled_geo, matrix, attrib, mask);
@@ -3081,6 +3146,10 @@
         WindowManager& wm = WindowManager::Default();
         wm.viewport_layout_changed.emit(screen->vpSize().width(), screen->vpSize().height());
       }
+      else if (strcmp(name, "close_window_key") == 0)
+      {
+        UpdateCloseWindowKey(v.action().key());
+      }
     }
   }
   return status;

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2013-01-22 20:15:54 +0000
+++ plugins/unityshell/src/unityshell.h	2013-02-01 03:24:22 +0000
@@ -236,7 +236,6 @@
   void OnInitiateSpread();
   void OnTerminateSpread();
 
-  void RestoreWindow(GVariant* data);
   bool SaveInputThenFocus(const guint xid);
 
   void OnPanelStyleChanged();
@@ -247,6 +246,11 @@
   bool TopPanelBackgroundTextureNeedsUpdate() const;
   void UpdateTopPanelBackgroundTexture();
 
+  unsigned CompizModifiersToNux(unsigned input) const;
+  unsigned XModifiersToNux(unsigned input) const;
+
+  void UpdateCloseWindowKey(CompAction::KeyBinding const&);
+
   std::unique_ptr<na::TickSource> tick_source_;
   std::unique_ptr<na::AnimationController> animation_controller_;
 

=== modified file 'unity-shared/WindowManager.h'
--- unity-shared/WindowManager.h	2012-12-03 15:34:23 +0000
+++ unity-shared/WindowManager.h	2013-02-01 03:24:22 +0000
@@ -26,6 +26,7 @@
 
 // To bring in nux::Geometry we first need the Rect header, then Utils.
 #include <NuxCore/Rect.h>
+#include <NuxCore/Property.h>
 #include <Nux/Utils.h>
 
 #ifdef USE_X11
@@ -150,6 +151,9 @@
 
   virtual std::string GetWindowName(Window window_id) const = 0;
 
+  // Nux Modifiers, Nux Keycode (= X11 KeySym)
+  nux::Property<std::pair<unsigned, unsigned>> close_window_key;
+
   // Signals
   sigc::signal<void, Window> window_mapped;
   sigc::signal<void, Window> window_unmapped;