Merge lp:~mhr3/unity/fix-856205 into lp:unity

[Michal Hruby]

Fixes bug #856205 by delaying activation of the first item in the model. Note that this requires also change to lenses (because currently they emit the SearchFinished signal before updating the model).

As didrocks informed me, this can't just go to stable branch without going to trunk, although I have much nicer fix in mind once we change some libunity internals.

[Mikkel Kamstrup Erlandsen (kamstrup)]

I tested this quite thoroughly and it seems to work well. Code looks good too. Although there are some nitpicks:

 i) in ResetSearchState() Why reinterpret_cast and not static_cast?

 ii) in ResetSearchState() should we not g_source_remove (searching_timeout_id_); if the id != 0?

review: Approve

[Tim Penhey (thumper)]

Inside the unity code, can we please stay away from glib types?

Just use "unsigned" instead of "guint".

[Mikkel Kamstrup Erlandsen (kamstrup)]

@Tim - that seems to be begging for trouble? This begs for 32/64 bit issues and nasty bool/gboolean issues both of which we've seen before.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Like the changes. Re-approved! :-)

review: Approve

[Tim Penhey (thumper)]

Due to lack of infrastructure for testing the timing issues here, I'm happy with a manual test.

review: Approve

=== added file 'manual-tests/Dash.txt'
--- manual-tests/Dash.txt	1970-01-01 00:00:00 +0000
+++ manual-tests/Dash.txt	2011-11-28 20:56:25 +0000
@@ -0,0 +1,13 @@
+Dash search
+-----------
+This test makes sure that the right command is run when you search
+using the dash. (see lp:856205)
+
+#. Press Alt+F2
+#. Press 'g'. Make sure you see some command name (like gcc)
+#. Quickly type 'edit<Enter>' - so you'd run 'gedit'.
+
+Outcome
+  The dash disappears, and gedit is run. If nothing happens or the first
+  command is run (in this case gcc), this test failed.
+

=== modified file 'plugins/unityshell/src/DashView.cpp'
--- plugins/unityshell/src/DashView.cpp	2011-11-13 22:57:27 +0000
+++ plugins/unityshell/src/DashView.cpp	2011-11-28 20:56:25 +0000
@@ -49,6 +49,9 @@
   : nux::View(NUX_TRACKER_LOCATION)
   , active_lens_view_(0)
   , last_activated_uri_("")
+  , searching_timeout_id_(0)
+  , search_in_progress_(false)
+  , activate_on_finish_(false)
   , visible_(false)
 
 {
@@ -69,6 +72,8 @@
 
 DashView::~DashView()
 {
+  if (searching_timeout_id_)
+    g_source_remove (searching_timeout_id_);
   delete bg_layer_;
   delete bg_darken_layer_;
 }
@@ -610,9 +615,32 @@
   QueueDraw();
 }
 
+gboolean DashView::ResetSearchStateCb(gpointer data)
+{
+  DashView *self = static_cast<DashView*>(data);
+
+  self->search_in_progress_ = false;
+  self->activate_on_finish_ = false;
+  self->searching_timeout_id_ = 0;
+
+  return FALSE;
+}
+
 void DashView::OnSearchChanged(std::string const& search_string)
 {
   LOG_DEBUG(logger) << "Search changed: " << search_string;
+  if (active_lens_view_)
+  {
+    search_in_progress_ = true;
+    // it isn't guaranteed that we get a SearchFinished signal, so we need
+    // to make sure this isn't set even though we aren't doing any search
+    if (searching_timeout_id_)
+    {
+      g_source_remove (searching_timeout_id_);
+    }
+    // 250ms for the Search method call, rest for the actual search
+    searching_timeout_id_ = g_timeout_add (500, &DashView::ResetSearchStateCb, this);
+  }
 }
 
 void DashView::OnLiveSearchReached(std::string const& search_string)
@@ -676,7 +704,12 @@
 void DashView::OnSearchFinished(std::string const& search_string)
 {
   if (search_bar_->search_string == search_string)
+  {
     search_bar_->SearchFinished();
+    search_in_progress_ = false;
+    if (activate_on_finish_)
+      this->OnEntryActivated();
+  }
 }
 
 void DashView::OnGlobalSearchFinished(std::string const& search_string)
@@ -785,7 +818,12 @@
 }
 void DashView::OnEntryActivated()
 {
-  active_lens_view_->ActivateFirst();
+  if (!search_in_progress_)
+  {
+    active_lens_view_->ActivateFirst();
+  }
+  // delay the activation until we get the SearchFinished signal
+  activate_on_finish_ = search_in_progress_;
 }
 
 // Keyboard navigation

=== modified file 'plugins/unityshell/src/DashView.h'
--- plugins/unityshell/src/DashView.h	2011-11-11 13:02:43 +0000
+++ plugins/unityshell/src/DashView.h	2011-11-28 20:56:25 +0000
@@ -103,6 +103,8 @@
 
   nux::Area* KeyNavIteration(nux::KeyNavDirection direction);
 
+  static gboolean ResetSearchStateCb(gpointer data);
+
 private:
   UBusManager ubus_manager_;
   FilesystemLenses lenses_;
@@ -130,6 +132,10 @@
   nux::ObjectPtr <nux::IOpenGLBaseTexture> bg_shine_texture_;
 
   std::string last_activated_uri_;
+  // we're passing this back to g_* functions, so we'll keep the g* type
+  guint searching_timeout_id_;
+  bool search_in_progress_;
+  bool activate_on_finish_;
 
   bool visible_;
 };