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

Proposed by Michal Hruby
Status: Merged
Approved by: Gord Allott
Approved revision: no longer in the source branch.
Merged at revision: 2206
Proposed branch: lp:~mhr3/unity/fix-966417
Merge into: lp:unity
Diff against target: 97 lines (+64/-0)
3 files modified
plugins/unityshell/src/DashView.cpp (+24/-0)
plugins/unityshell/src/DashView.h (+1/-0)
tests/autopilot/autopilot/tests/test_home_lens.py (+39/-0)
To merge this branch: bzr merge lp:~mhr3/unity/fix-966417
Reviewer Review Type Date Requested Status
Gord Allott (community) Approve
Thomi Richards (community) Needs Fixing
Review via email: mp+100138@code.launchpad.net

Commit message

Make sure applications can be started from the HomeLens as soon as we get results from apps lens

Description of the change

All the lenses are currently waiting for results before one can execute an item - this is to prevent issues where you quickly type for example "gedit" in the dash and since the results might not be available at the time when you press enter (but they will a couple of milliseconds later) we could launch incorrect item if we didn't wait.
Unfortunately this is a huge problem particularly in the home lens which aggregates results from multiple lenses, so even though you see gedit being in the result set, unity won't launch it, because it's waiting for the files lens to finish searching.

To circumvent the issue and still maintain some determinism, the home lens is now waiting for results from the apps lens before launching an item. (if we just waited for results from any lens, what could happen is that sometimes gedit would launch gedit while other times gedit would launch firefox with a tweet from someone mentioning gedit - a possible result from gwibber lens)

Added AP test for the issue.

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

Hi,

Please remove the following imports, as you don't need them:

67 +from subprocess import call
70 +from autopilot.emulators.bamf import Bamf

You don't need this:

80 + self.dash = Dash()

...as it's done already in the AutopilotTestCase. This means you also don't need to import the Dash.

You also don't need this:

93 + kb = Keyboard()

use self.keyboard instead - this means you don't need the Keyboard import.

Finally, I have two really small requests:

 1) Please change the author name in the copyright header :)
 2) Docstrings for tests should avoid words like 'should', in favour of stronger words like 'must'. How about something like: """Hitting enter must run an application even though the search has not finished yet."""

Apart from the issues above, nice test - thanks!
Cheers,

review: Needs Fixing
Revision history for this message
Michal Hruby (mhr3) wrote :

And... fixed

Revision history for this message
Gord Allott (gordallott) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/unityshell/src/DashView.cpp'
--- plugins/unityshell/src/DashView.cpp 2012-03-29 18:06:53 +0000
+++ plugins/unityshell/src/DashView.cpp 2012-04-02 07:59:18 +0000
@@ -457,6 +457,14 @@
457 lens->activated.connect(sigc::mem_fun(this, &DashView::OnUriActivatedReply));457 lens->activated.connect(sigc::mem_fun(this, &DashView::OnUriActivatedReply));
458 lens->search_finished.connect(sigc::mem_fun(this, &DashView::OnSearchFinished));458 lens->search_finished.connect(sigc::mem_fun(this, &DashView::OnSearchFinished));
459 // global search done is handled by the home lens, no need to connect to it459 // global search done is handled by the home lens, no need to connect to it
460 // BUT, we will special case global search finished coming from
461 // the applications lens, because we want to be able to launch applications
462 // immediately without waiting for the search finished signal which will
463 // be delayed by all the lenses we're searching
464 if (id == "applications.lens")
465 {
466 lens->global_search_finished.connect(sigc::mem_fun(this, &DashView::OnAppsGlobalSearchFinished));
467 }
460}468}
461469
462void DashView::OnLensBarActivated(std::string const& id)470void DashView::OnLensBarActivated(std::string const& id)
@@ -531,6 +539,22 @@
531 OnSearchFinished(hints);539 OnSearchFinished(hints);
532}540}
533541
542void DashView::OnAppsGlobalSearchFinished(Lens::Hints const& hints)
543{
544 if (active_lens_view_ == home_view_)
545 {
546 /* HACKITY HACK! We're resetting the state of search_in_progress when
547 * doing searches in the home lens and we get results from apps lens.
548 * This way typing a search query and pressing enter immediately will
549 * wait for the apps lens results and will run correct application.
550 * See lp:966417 and lp:856206 for more info about why we do this.
551 */
552 search_in_progress_ = false;
553 if (activate_on_finish_)
554 this->OnEntryActivated();
555 }
556}
557
534void DashView::OnUriActivated(std::string const& uri)558void DashView::OnUriActivated(std::string const& uri)
535{559{
536 last_activated_uri_ = uri;560 last_activated_uri_ = uri;
537561
=== modified file 'plugins/unityshell/src/DashView.h'
--- plugins/unityshell/src/DashView.h 2012-03-21 14:18:06 +0000
+++ plugins/unityshell/src/DashView.h 2012-04-02 07:59:18 +0000
@@ -90,6 +90,7 @@
90 void OnLensBarActivated(std::string const& id);90 void OnLensBarActivated(std::string const& id);
91 void OnSearchFinished(Lens::Hints const& hints);91 void OnSearchFinished(Lens::Hints const& hints);
92 void OnGlobalSearchFinished(Lens::Hints const& hints);92 void OnGlobalSearchFinished(Lens::Hints const& hints);
93 void OnAppsGlobalSearchFinished(Lens::Hints const& hints);
93 void OnUriActivated(std::string const& uri);94 void OnUriActivated(std::string const& uri);
94 void OnUriActivatedReply(std::string const& uri, HandledType type, Lens::Hints const&);95 void OnUriActivatedReply(std::string const& uri, HandledType type, Lens::Hints const&);
95 bool DoFallbackActivation(std::string const& uri);96 bool DoFallbackActivation(std::string const& uri);
9697
=== added file 'tests/autopilot/autopilot/tests/test_home_lens.py'
--- tests/autopilot/autopilot/tests/test_home_lens.py 1970-01-01 00:00:00 +0000
+++ tests/autopilot/autopilot/tests/test_home_lens.py 2012-04-02 07:59:18 +0000
@@ -0,0 +1,39 @@
1# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
2# Copyright 2012 Canonical
3# Author: Michal Hruby
4#
5# This program is free software: you can redistribute it and/or modify it
6# under the terms of the GNU General Public License version 3, as published
7# by the Free Software Foundation.
8
9from time import sleep
10
11from autopilot.emulators.unity.dash import Dash
12from autopilot.tests import AutopilotTestCase
13
14
15class HomeLensSearchTests(AutopilotTestCase):
16 """Test the command lense search bahavior."""
17
18 def setUp(self):
19 super(HomeLensSearchTests, self).setUp()
20
21 def tearDown(self):
22 self.dash.ensure_hidden()
23 super(HomeLensSearchTests, self).tearDown()
24
25 def test_quick_run_app(self):
26 """Hitting enter runs an application even though a search might not have fully finished yet."""
27 if self.app_is_running("Text Editor"):
28 self.close_all_app("Text Editor")
29 sleep(1)
30
31 kb = self.keyboard
32 self.dash.ensure_visible()
33 kb.type("g")
34 sleep(1)
35 kb.type("edit", 0.1)
36 kb.press_and_release("Enter", 0.1)
37 self.addCleanup(self.close_all_app, "Text Editor")
38 app_found = self.bamf.wait_until_application_is_running("gedit.desktop", 5)
39 self.assertTrue(app_found)