Merge lp:~lore-mattei/unity/fix-bug-891818 into lp:unity

[Lorenzo Mattei]

[Mirco Müller (macslow)]

While your fix certainly does fix the stated issue, it is missing an autopilot-test to help strengthen the test-harness for unity, which helps us avoid future regressions for bugs that already have been fixed. This is part of our software-engineering QA.

And finally there's some "paperwork" needed before your branch can finally make it into trunk. You need to sign the contributor agreement and send it to the person managing this for unity. Please go to http://www.canonical.com/contributors for all the details.

Thanks sofar for you time and efforts!

review: Needs Fixing

[Mirco Müller (macslow)]

Thanks for signing the paperwork. Now with the added test this is good to go. Well done, Lorenzo!

review: Approve

[Paweł Stołowski (stolowski)]

I'm working on fixing this problem in unity-2d and there seems to be a generic consensus in unity-2d to avoid harcoding ALT+F4 and picking the key binding from window manager configuration instead. You may want to consider this.

[Marco Trevisan (Treviño) (3v1n0)]

> I'm working on fixing this problem in unity-2d and there seems to be a generic
> consensus in unity-2d to avoid harcoding ALT+F4 and picking the key binding
> from window manager configuration instead. You may want to consider this.

Yes, that's true. On this case, the fix should be included into Nux itself, anyway for now this is a good local fix.

44 + """Dash must close on alt+F4."""
45 + self.dash.ensure_visible()

This is fine, but you'd also need to add after:

 self.addCleanup(self.dash.ensure_hidden)

This will ensure that if the test fails, the dash is hidden anyway.
Maybe, even including a small sleep after the alt+f4 press, could avoid false-negatives.

review: Needs Fixing

[Lorenzo Mattei (lore-mattei)]

> This is fine, but you'd also need to add after:
>
> self.addCleanup(self.dash.ensure_hidden)
>
> This will ensure that if the test fails, the dash is hidden anyway.

I think the self.dash.ensure_hidden() call in tearDown() will ensure that. Isn't it called even in case of fail?

> Maybe, even including a small sleep after the alt+f4 press, could avoid false-
> negatives.
Ok.

[Marco Trevisan (Treviño) (3v1n0)]

> > This is fine, but you'd also need to add after:
> >
> > self.addCleanup(self.dash.ensure_hidden)
> >
> > This will ensure that if the test fails, the dash is hidden anyway.
>
> I think the self.dash.ensure_hidden() call in tearDown() will ensure that.
> Isn't it called even in case of fail?

Yes, sure. The version I checked, maybe was missing it.
Sorry.

review: Approve

[Paweł Stołowski (stolowski)]

> I'm working on fixing this problem in unity-2d and there seems to be a generic
> consensus in unity-2d to avoid harcoding ALT+F4 and picking the key binding
> from window manager configuration instead. You may want to consider this.

We decided to hardcode it for now in unity-2d after all, as it turned out require a lot of work to convert between Metacity and Qt, and it's not feasible right now.

=== modified file 'plugins/unityshell/src/DashView.cpp'
--- plugins/unityshell/src/DashView.cpp	2012-02-14 10:18:15 +0000
+++ plugins/unityshell/src/DashView.cpp	2012-03-07 14:05:19 +0000
@@ -713,6 +713,14 @@
     // Not sure if Enter should be a navigation key
     direction = KEY_NAV_ENTER;
     break;
+  case NUX_VK_F4:
+    // Maybe we should not do it here, but it needs to be checked where 
+    // we are able to know if alt is pressed.
+    if (special_keys_state & NUX_STATE_ALT)
+    {
+      ubus_manager_.SendMessage(UBUS_PLACE_VIEW_CLOSE_REQUEST);
+    }
+    break;
   default:
     direction = KEY_NAV_NONE;
     break;

=== modified file 'tests/autopilot/autopilot/emulators/unity/dash.py'
--- tests/autopilot/autopilot/emulators/unity/dash.py	2012-02-29 22:00:01 +0000
+++ tests/autopilot/autopilot/emulators/unity/dash.py	2012-03-07 14:05:19 +0000
@@ -115,6 +115,10 @@
         active_lens_name = self.view.get_lensbar().active_lens
         return self.view.get_lensview_by_name(active_lens_name)
 
+    def close_with_alt_f4(self):
+        """Send ALT+F4 in order to close the dash."""
+        self._keyboard.press_and_release("Alt+F4")
+
 
 class DashController(UnityIntrospectionObject):
     """The main dash controller object."""

=== modified file 'tests/autopilot/autopilot/tests/test_dash.py'
--- tests/autopilot/autopilot/tests/test_dash.py	2012-03-06 18:21:27 +0000
+++ tests/autopilot/autopilot/tests/test_dash.py	2012-03-07 14:05:19 +0000
@@ -71,6 +71,13 @@
         self.dash.reveal_command_lens()
         lensbar = self.dash.view.get_lensbar()
         self.assertEqual(lensbar.active_lens, u'commands.lens')
+    
+    def test_alt_f4_close_dash(self):
+        """Dash must close on alt+F4."""
+        self.dash.ensure_visible()
+        self.dash.close_with_alt_f4()
+        sleep(0.5)
+        self.assertFalse(self.dash.get_is_visible())
 
 
 class DashKeyNavTests(AutopilotTestCase):