Unity

Merge lp:~attente/unity/gnome-key-grabber into lp:unity

  1. gnome-key-grabber
  2. Merge into trunk
Proposed by William Hua
Status: Merged
Approved by: Christopher Townsend
Approved revision: no longer in the source branch.
Merged at revision: 3650
Proposed branch: lp:~attente/unity/gnome-key-grabber
Merge into: lp:unity
Diff against target: 1070 lines (+774/-15)
14 files modified
panel/PanelController.cpp (+127/-5)
panel/PanelController.h (+5/-0)
panel/PanelView.cpp (+6/-7)
panel/PanelView.h (+1/-0)
plugins/unityshell/src/unityshell.cpp (+35/-1)
plugins/unityshell/src/unityshell.h (+9/-0)
plugins/unityshell/unityshell.xml.in (+7/-1)
tests/autopilot/unity/tests/test_gnome_key_grabber.py (+173/-0)
tests/autopilot/unity/tests/test_panel.py (+15/-0)
unity-shared/CMakeLists.txt (+9/-1)
unity-shared/GnomeKeyGrabber.cpp (+268/-0)
unity-shared/GnomeKeyGrabber.h (+54/-0)
unity-shared/GnomeKeyGrabberImpl.h (+64/-0)
unity-standalone/CMakeLists.txt (+1/-0)
To merge this branch: bzr merge lp:~attente/unity/gnome-key-grabber
Reviewer Review Type Date Requested Status
Christopher Townsend Approve
Marco Trevisan (Treviño) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+202755@code.launchpad.net

Commit message

Implement the GNOME key grabber interface so that Compiz and gnome-settings-daemon no longer have to fight for key grabs. Also, fix the global menu bar mnemonics. (LP: #1113008, LP: #1206582, LP: #1226962)

Description of the change

(This branch depends on https://code.launchpad.net/~attente/compiz/plugin-actions/+merge/200307)

Implement the GNOME key grabber interface so that Compiz and gnome-settings-daemon no longer have to fight for key grabs. Also, fix the global menu bar mnemonics. (LP: #1113008, LP: #1206582, LP: #1226962)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3620
http://jenkins.qa.ubuntu.com/job/unity-ci/564/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/100/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/96/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/98/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Hi William,

We are going to wait to review this until we get https://code.launchpad.net/~3v1n0/unity/unity-decorations/+merge/202582 merged into trunk. There will likely be some conflicts, so those will need to be fixed once that merges.

Thanks!

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :
Download full text (3.5 KiB)

+bool PanelMenuView::SetMenuBarVisible(bool visible)
75 +{
76 + menu_bar_visible_ = visible;
77 + QueueDraw();
78 + return true;
79 +}
80 +

Only queuedraw and return true if value really changed (likely to happen, but nicer to see :)).

Also, in PanelMenuView you added a new var "menu_bar_visible_"... We already had show_now_activated_ for this purpose... I'm not sure if them might clash here, but what about just using one?

Also, to make the panel show the menu items correctly underlined, each PanelIndicatorEntry should have the show-now state...

251 + current_action_id_++;
Prefixed increment?

261 + CompAction& added(actions_.back());

Mnh, this is needed to access to the reference of it?
As I'd just use action instead... But maybe compiz definitions makes things harder.

262 + if (grabs_by_binding_[added.key()]++ == 0)
263 + screen_->addAction(&added);

A little hard to read... Probably using (with proper spacing):
auto& grab = grabs_by_binding_[added.key()];
if (grab == 0) { screen_->addAction(&added) }
++grab;

270 + std::map<const CompAction*, unsigned int>::const_iterator i(action_ids_by_action_.find(&action));

Auto is your friend here...
auto i = action_ids_by_action_.find(&action);
And everywhere we have iterators or complex or duplicated types (like A = new A()), just use it.

283 + if (--grabs_by_binding_[j->key()] == 0)
284 + screen_->removeAction(&*j);

As before... Also this &* thing is not the best to see...
248 +unsigned int GnomeKeyGrabber::Impl::addAction(const CompAction& action,
249 + bool addressable)

As for general style, we generally try to keep method names (with parameters) in just one line, unless it really becomes too long (where "too long" is not exactly specified) :P
Also we use Type const&, not const Type&...

315 + g_variant_builder_init(&builder, G_VARIANT_TYPE("au"));

This is, fine... But for your convenience you can use glib::Variant::FromVector for generating arrays such this...

action.setInitiate(boost::bind(&GnomeKeyGrabber::Impl::actionInitiated, this, _1, _2, _3));

std::bind? Or probably you can just use lambdas here, as they improve readability for small cases such these ones.

568 + std::map<const CompAction*, unsigned int> action_ids_by_action_;
569 + std::map<unsigned int, const CompAction*> actions_by_action_id_;

Mh, I'm not a fan of using non-smart pointers in stl containers, even though action_ vector is the owner... Could be possible to use something better than pure pointers here?

Can you also unit-tests for GnomeKeyGrabber (in the same way I did for GnomeSessionManager)?

Also I'd move the whole KeyGrabber thing on a separate library/folder... That links with unity-shared-compiz.

938 + priv->info_by_entry = g_hash_table_new_full (NULL, NULL, NULL, action_info_free);

720 + <option name="show_menu_bar" type="key">

Since this will clash with Hud reveal key, could you include in both descriptions that one is a "Tap", while the other is a key-press?

808 + PanelService *service;

As panelservice uses a singleto, I think you don't need to pass the service to every entry... Just use the global instance.

938 + priv->info_by_entry = g_hash_table_new_full (NULL, NULL, NULL, action_info...

Read more...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3622
http://jenkins.qa.ubuntu.com/job/unity-ci/577/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/113
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/109
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/111

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3631
http://jenkins.qa.ubuntu.com/job/unity-ci/585/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/121
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/117
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/119

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3633
http://jenkins.qa.ubuntu.com/job/unity-ci/586/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/120

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3634
http://jenkins.qa.ubuntu.com/job/unity-ci/589/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/125
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/121
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/123

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3636
http://jenkins.qa.ubuntu.com/job/unity-ci/591/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/127/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/123/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/125/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
William Hua (attente) wrote :

Hi Chris and Marco,

Thanks for the comments, I've resolved most of them. The MP is quite a bit smaller too, thanks for that!

> 261 + CompAction& added(actions_.back());
>
> Mnh, this is needed to access to the reference of it?
> As I'd just use action instead... But maybe compiz definitions makes things
> harder.

Yes, you're correct, we must pass a non-const pointer to compiz.

> 315 + g_variant_builder_init(&builder, G_VARIANT_TYPE("au"));
>
> This is, fine... But for your convenience you can use
> glib::Variant::FromVector for generating arrays such this...

Thanks, I'm leaving this as is since we need it to type properly if we get an empty array. Also, it might be more efficient than constructing an intermediate vector.

> 568 + std::map<const CompAction*, unsigned int> action_ids_by_action_;
> 569 + std::map<unsigned int, const CompAction*> actions_by_action_id_;
>
> Mh, I'm not a fan of using non-smart pointers in stl containers, even though
> action_ vector is the owner... Could be possible to use something better than
> pure pointers here?

So, while not as nice looking, I think I can justify it for a couple of reasons. We never end up de-referencing those pointers; their use is only for lookups of action ids. Also, I don't want the key grabber to keep those objects alive with a shared_ptr, and using weak_ptrs might have bad effects with the map when they get zeroed out.

> Can you also unit-tests for GnomeKeyGrabber (in the same way I did for
> GnomeSessionManager)?

I added a few autopilot tests for this.

> However, as for general design thing... Whouldn't be possible to handle the
> Activation, and registration of actions inside unity itself (instead of doing
> the work by dbus)? I know it's the standard way at this point, but maybe it
> would have kept the panel service simpler...

Thanks, moved it into the panel implementation instead :)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

110 + auto accelerator(gtk_accelerator_name(gdk_keyval_to_lower(gdk_unicode_to_keyval(mnemonic)), GDK_MOD1_MASK));

use glib::String here.

111 + auto action(std::make_shared<CompAction>());

Why not just auto action = std::make_shared... ? g++ would just use the best way to initialize it. Same in other places...

115 + action->setInitiate([=](CompAction* action, CompAction::State state, CompOption::Vector& options)
116 + {

Just pass [this, id] to the lambda, where id is the entry->id() copied to a string.

1061 + CompScreen* screen_;
FYI CompScreen is exported by <core/screen.h> as a global "screen" variable and is always available since the plugin initialization, so you can just use that reference if you want.

884 + action.setInitiate([=]
892 + action.setTerminate([=]

It doesn't seem you need to pass anything a part from [this] to these lambdas, so just avoid to copy everything please.

1066 + std::map<CompAction const*, unsigned int> action_ids_by_action_;
1067 + std::map<unsigned int, CompAction const*> actions_by_action_id_;

As you're never iterating over them, using std::unordered_map here might give us some benefit.

Revision history for this message
William Hua (attente) wrote :

Thanks Marco! Cleaned up those issues.

> 1061 + CompScreen* screen_;
> FYI CompScreen is exported by <core/screen.h> as a global "screen" variable
> and is always available since the plugin initialization, so you can just use
> that reference if you want.

Thanks for the heads up, personally, I don't mind passing it down to the grabber.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

So, I've just tested in the real world, and it works very well...

However, I'm getting some console errors we should avoid:

ERROR 2014-02-04 19:44:10 unityfree <unknown>:0 the GVariant format string '(b)' has a type of '(b)' but the given value has a type of 'b'
ERROR 2014-02-04 19:44:10 unityfree <unknown>:0 g_variant_get: assertion 'valid_format_string (format_string, TRUE, value)' failed

I'm not sure where this is happening...

Also, there's a small regression: on Alt pressure, the menu entries should show after a small delay. The logic is already handled by PanelMenuView, so using your code just replacing your PanelMenuView::SetMenuBarVisible with http://pastebin.ubuntu.com/6874592/ is enough.

However, imho you don't need to be so invasive... and you can just use this way: http://pastebin.ubuntu.com/6874662/ (apply this to your branch)

Revision history for this message
Christopher Townsend (townsend) wrote :

Hi William,

I've run the AP tests and I'm getting a failure for the unity.tests.test_gnome_key_grabber.GnomeKeyGrabberTests.test_grab_accelerator test. Here is the AP log of the failure:

Traceback (most recent call last):
  File "/home/townsend/Reviews/misc/unity/tests/autopilot/unity/tests/test_gnome_key_grabber.py", line 87, in test_grab_accelerator
    self.check_accelerator(action, 'Shift+Control+Alt+a')
  File "/home/townsend/Reviews/misc/unity/tests/autopilot/unity/tests/test_gnome_key_grabber.py", line 50, in check_accelerator
    self.assertTrue(activated[0])
  File "/usr/lib/python2.7/unittest/case.py", line 424, in assertTrue
    raise self.failureException(msg)
AssertionError: False is not true

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3638
http://jenkins.qa.ubuntu.com/job/unity-ci/594/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/130
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/126
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/128

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

review: Approve (continuous-integration)
Revision history for this message
William Hua (attente) wrote :

> However, I'm getting some console errors we should avoid:
>
> ERROR 2014-02-04 19:44:10 unityfree <unknown>:0 the GVariant format string
> '(b)' has a type of '(b)' but the given value has a type of 'b'
> ERROR 2014-02-04 19:44:10 unityfree <unknown>:0 g_variant_get: assertion
> 'valid_format_string (format_string, TRUE, value)' failed
>
> I'm not sure where this is happening...

It's something unrelated in trunk it seems, here's an independent fix:

https://code.launchpad.net/~attente/unity/hud-controller-gvariant-type-error/+merge/204893

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3641
http://jenkins.qa.ubuntu.com/job/unity-ci/598/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/130
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/132

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

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Hmm, I'm still getting the same AP test failure as I mentioned above.

Also, it's probably better to leave out the print's in your AP code. If you really need to capture that output, then perhaps having it output when --verbose is passed to autopilot would be better.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3642
http://jenkins.qa.ubuntu.com/job/unity-ci/601/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/133
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/135

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

review: Approve (continuous-integration)
Revision history for this message
William Hua (attente) wrote :

Hi Christopher, can you re-run the tests and paste the output? I'm unable to replicate the problem.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3643
http://jenkins.qa.ubuntu.com/job/unity-ci/604/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/140
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/138

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

review: Approve (continuous-integration)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

I don't get failures as well.

However, speaking of the code itself, I'm ok with it now.

As for testing this AP code sees a little too complex: I mean, it's better to keep single tests small adding more functions. Also, since we're in the AP land, it would be nice if you might test the real world usage (i.e. open a window, better a mocked one, and test that it's menus are shown on Alt or opened on accelerators).
In general I prefer unit tests (especially for non-visual things like this), and in this case you might have done them quite easily.

Revision history for this message
William Hua (attente) wrote :

Even though the tests are non-visual, I think it's easiest for us to test the key presses and releases using AP.

Unity already has AP tests for the panel that are currently failing, no? I saw them under unity.tests.test_panel.PanelKeyNavigationTests, and running them I only get a test failure for test_panel_first_menu_show_works, but it seems to be for the Alt+F10 key which is unrelated to this MP.

Revision history for this message
William Hua (attente) wrote :

Sorry, on second glance, I see only an "Alt+F" test, I don't see one for holding "Alt". I'll add another panel AP test for that.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3644
http://jenkins.qa.ubuntu.com/job/unity-ci/606/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/142
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/138
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/140

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

review: Approve (continuous-integration)
Revision history for this message
William Hua (attente) wrote :

Hi Christopher, I think the reason the new AP tests are failing is the same reason why this MP exists in the first place: compiz and g-s-d are still fighting over the key grabs. You need the g-s-d from this branch to fully fix the bug and have the AP tests working again: https://code.launchpad.net/~attente/gnome-settings-daemon/gnome-key-grabber.

I uploaded that g-s-d to a PPA so you can just download the package from here without building that branch: https://launchpad.net/~attente/+archive/gnome-key-grabber.

Revision history for this message
Christopher Townsend (townsend) wrote :

Hi William,

I installed your PPA's g-s-d and I still get the failure for that same particular test:(

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3645
http://jenkins.qa.ubuntu.com/job/unity-ci/608/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/144
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/140
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/142

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3646
http://jenkins.qa.ubuntu.com/job/unity-ci/610/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-amd64-ci/146
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-armhf-ci/142
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-trusty-i386-ci/144

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

review: Approve (continuous-integration)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ok, things are working well here, so I think we can safely approve it.

As for more tests, feel free to propose other branches. :)

review: Approve
Revision history for this message
Christopher Townsend (townsend) wrote :

Hi William,

Before we globally approve, you'll need to merge trunk again as there is now a conflict in test_panel.py.

Once you do that, we will go ahead and merge since I'm the only one getting the test_grab_accelerator issue. Brandon tried as well and it passes for him, so I think there is probably some issue with my test machine. At any rate, I'll keep an eye on the daily-build Jenkins CI runs to make sure it passes there as well.

Revision history for this message
Christopher Townsend (townsend) wrote :

Ok, approving this as well. Let's get it in!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'panel/PanelController.cpp'
--- panel/PanelController.cpp 2014-01-25 13:39:49 +0000
+++ panel/PanelController.cpp 2014-02-11 20:09:36 +0000
@@ -39,9 +39,15 @@
39class Controller::Impl39class Controller::Impl
40{40{
41public:41public:
42 Impl(ui::EdgeBarrierController::Ptr const& edge_barriers);42 Impl(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber);
43 ~Impl();43 ~Impl();
4444
45 void GrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator);
46 void UngrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator);
47 void GrabEntryMnemonics(indicator::Entry::Ptr const& entry);
48 void UngrabEntryMnemonics(std::string const& entry_id);
49
50 void SetMenuBarVisible(bool visible);
45 void FirstMenuShow();51 void FirstMenuShow();
46 void QueueRedraw();52 void QueueRedraw();
4753
@@ -64,6 +70,7 @@
64 PanelView* ViewForWindow(BaseWindowPtr const& window) const;70 PanelView* ViewForWindow(BaseWindowPtr const& window) const;
6571
66 ui::EdgeBarrierController::Ptr edge_barriers_;72 ui::EdgeBarrierController::Ptr edge_barriers_;
73 GnomeKeyGrabber::Ptr grabber_;
67 PanelVector panels_;74 PanelVector panels_;
68 std::vector<nux::Geometry> panel_geometries_;75 std::vector<nux::Geometry> panel_geometries_;
69 std::vector<Window> tray_xids_;76 std::vector<Window> tray_xids_;
@@ -75,11 +82,15 @@
75 int menus_discovery_fadein_;82 int menus_discovery_fadein_;
76 int menus_discovery_fadeout_;83 int menus_discovery_fadeout_;
77 indicator::DBusIndicators::Ptr dbus_indicators_;84 indicator::DBusIndicators::Ptr dbus_indicators_;
85 std::unordered_map<std::string, std::shared_ptr<CompAction>> entry_actions_;
86 connection::Manager dbus_indicators_connections_;
87 std::unordered_map<indicator::Indicator::Ptr, connection::Manager> indicator_connections_;
78};88};
7989
8090
81Controller::Impl::Impl(ui::EdgeBarrierController::Ptr const& edge_barriers)91Controller::Impl::Impl(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber)
82 : edge_barriers_(edge_barriers)92 : edge_barriers_(edge_barriers)
93 , grabber_(grabber)
83 , opacity_(1.0f)94 , opacity_(1.0f)
84 , opacity_maximized_toggle_(false)95 , opacity_maximized_toggle_(false)
85 , menus_fadein_(0)96 , menus_fadein_(0)
@@ -88,10 +99,28 @@
88 , menus_discovery_fadein_(0)99 , menus_discovery_fadein_(0)
89 , menus_discovery_fadeout_(0)100 , menus_discovery_fadeout_(0)
90 , dbus_indicators_(std::make_shared<indicator::DBusIndicators>())101 , dbus_indicators_(std::make_shared<indicator::DBusIndicators>())
91{}102{
103 if (grabber_)
104 {
105 for (auto const& indicator : dbus_indicators_->GetIndicators())
106 GrabIndicatorMnemonics(indicator);
107
108 auto& connections = dbus_indicators_connections_;
109 connections.Add(dbus_indicators_->on_object_added.connect(sigc::mem_fun(this, &Impl::GrabIndicatorMnemonics)));
110 connections.Add(dbus_indicators_->on_object_removed.connect(sigc::mem_fun(this, &Impl::UngrabIndicatorMnemonics)));
111 }
112}
92113
93Controller::Impl::~Impl()114Controller::Impl::~Impl()
94{115{
116 if (grabber_)
117 {
118 dbus_indicators_connections_.Clear();
119
120 for (auto const& indicator : dbus_indicators_->GetIndicators())
121 UngrabIndicatorMnemonics(indicator);
122 }
123
95 // Since the panels are in a window which adds a reference to the124 // Since the panels are in a window which adds a reference to the
96 // panel, we need to make sure the base windows are unreferenced125 // panel, we need to make sure the base windows are unreferenced
97 // otherwise the pnales never die.126 // otherwise the pnales never die.
@@ -102,6 +131,70 @@
102 }131 }
103}132}
104133
134void Controller::Impl::GrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator)
135{
136 if (indicator->IsAppmenu())
137 {
138 for (auto const& entry : indicator->GetEntries())
139 GrabEntryMnemonics(entry);
140
141 auto& connections = indicator_connections_[indicator];
142 connections.Add(indicator->on_entry_added.connect(sigc::mem_fun(this, &Impl::GrabEntryMnemonics)));
143 connections.Add(indicator->on_entry_removed.connect(sigc::mem_fun(this, &Impl::UngrabEntryMnemonics)));
144 }
145}
146
147void Controller::Impl::UngrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator)
148{
149 if (indicator->IsAppmenu())
150 {
151 indicator_connections_.erase(indicator);
152
153 for (auto const& entry : indicator->GetEntries())
154 UngrabEntryMnemonics(entry->id());
155 }
156}
157
158void Controller::Impl::GrabEntryMnemonics(indicator::Entry::Ptr const& entry)
159{
160 gunichar mnemonic;
161
162 if (pango_parse_markup(entry->label().c_str(), -1, '_', nullptr, nullptr, &mnemonic, nullptr) && mnemonic)
163 {
164 auto key = gdk_keyval_to_lower(gdk_unicode_to_keyval(mnemonic));
165 glib::String accelerator(gtk_accelerator_name(key, GDK_MOD1_MASK));
166 auto action = std::make_shared<CompAction>();
167 auto id = entry->id();
168
169 action->keyFromString(accelerator);
170 action->setState(CompAction::StateInitKey);
171 action->setInitiate([this, id](CompAction* action, CompAction::State state, CompOption::Vector& options)
172 {
173 for (auto const& panel : panels_)
174 {
175 if (panel->ActivateEntry(id))
176 return true;
177 }
178
179 return false;
180 });
181
182 entry_actions_[id] = action;
183 grabber_->addAction(*action);
184 }
185}
186
187void Controller::Impl::UngrabEntryMnemonics(std::string const& entry_id)
188{
189 auto i = entry_actions_.find(entry_id);
190
191 if (i != entry_actions_.end())
192 {
193 grabber_->removeAction(*i->second);
194 entry_actions_.erase(i);
195 }
196}
197
105void Controller::Impl::UpdatePanelGeometries()198void Controller::Impl::UpdatePanelGeometries()
106{199{
107 panel_geometries_.reserve(panels_.size());200 panel_geometries_.reserve(panels_.size());
@@ -112,6 +205,20 @@
112 }205 }
113}206}
114207
208void Controller::Impl::SetMenuBarVisible(bool visible)
209{
210 for (auto const& indicator : dbus_indicators_->GetIndicators())
211 {
212 if (indicator->IsAppmenu())
213 {
214 for (auto const& entry : indicator->GetEntries())
215 entry->set_show_now(visible);
216
217 break;
218 }
219 }
220}
221
115void Controller::Impl::FirstMenuShow()222void Controller::Impl::FirstMenuShow()
116{223{
117 for (auto const& panel: panels_)224 for (auto const& panel: panels_)
@@ -279,9 +386,9 @@
279 return opacity_;386 return opacity_;
280}387}
281388
282Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers)389Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber)
283 : launcher_width(64)390 : launcher_width(64)
284 , pimpl(new Impl(edge_barriers))391 , pimpl(new Impl(edge_barriers, grabber))
285{392{
286 UScreen* screen = UScreen::GetDefault();393 UScreen* screen = UScreen::GetDefault();
287 screen->changed.connect(sigc::mem_fun(this, &Controller::OnScreenChanged));394 screen->changed.connect(sigc::mem_fun(this, &Controller::OnScreenChanged));
@@ -293,11 +400,26 @@
293 });400 });
294}401}
295402
403Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers)
404 : Controller(edge_barriers, GnomeKeyGrabber::Ptr())
405{
406}
407
296Controller::~Controller()408Controller::~Controller()
297{409{
298 delete pimpl;410 delete pimpl;
299}411}
300412
413void Controller::ShowMenuBar()
414{
415 pimpl->SetMenuBarVisible(true);
416}
417
418void Controller::HideMenuBar()
419{
420 pimpl->SetMenuBarVisible(false);
421}
422
301void Controller::FirstMenuShow()423void Controller::FirstMenuShow()
302{424{
303 pimpl->FirstMenuShow();425 pimpl->FirstMenuShow();
304426
=== modified file 'panel/PanelController.h'
--- panel/PanelController.h 2014-01-25 13:39:49 +0000
+++ panel/PanelController.h 2014-02-11 20:09:36 +0000
@@ -24,7 +24,9 @@
24#include <Nux/Nux.h>24#include <Nux/Nux.h>
2525
26#include "launcher/EdgeBarrierController.h"26#include "launcher/EdgeBarrierController.h"
27#include "unity-shared/GnomeKeyGrabber.h"
27#include "unity-shared/Introspectable.h"28#include "unity-shared/Introspectable.h"
29
28namespace unity30namespace unity
29{31{
30namespace panel32namespace panel
@@ -37,9 +39,12 @@
37 typedef std::shared_ptr<Controller> Ptr;39 typedef std::shared_ptr<Controller> Ptr;
38 typedef std::vector<nux::ObjectPtr<PanelView>> PanelVector;40 typedef std::vector<nux::ObjectPtr<PanelView>> PanelVector;
3941
42 Controller(ui::EdgeBarrierController::Ptr const& barrier_controller, GnomeKeyGrabber::Ptr const& grabber);
40 Controller(ui::EdgeBarrierController::Ptr const& barrier_controller);43 Controller(ui::EdgeBarrierController::Ptr const& barrier_controller);
41 ~Controller();44 ~Controller();
4245
46 void ShowMenuBar();
47 void HideMenuBar();
43 void FirstMenuShow();48 void FirstMenuShow();
44 void QueueRedraw();49 void QueueRedraw();
4550
4651
=== modified file 'panel/PanelView.cpp'
--- panel/PanelView.cpp 2014-02-06 22:12:59 +0000
+++ panel/PanelView.cpp 2014-02-11 20:09:36 +0000
@@ -592,13 +592,7 @@
592592
593void PanelView::OnEntryActivateRequest(std::string const& entry_id)593void PanelView::OnEntryActivateRequest(std::string const& entry_id)
594{594{
595 if (!IsActive())595 ActivateEntry(entry_id);
596 return;
597
598 bool ret;
599
600 ret = menu_view_->ActivateEntry(entry_id, 0);
601 if (!ret) indicators_->ActivateEntry(entry_id, 0);
602}596}
603597
604bool PanelView::TrackMenuPointer()598bool PanelView::TrackMenuPointer()
@@ -690,6 +684,11 @@
690 return ret;684 return ret;
691}685}
692686
687bool PanelView::ActivateEntry(std::string const& entry_id)
688{
689 return IsActive() && (menu_view_->ActivateEntry(entry_id, 0) || indicators_->ActivateEntry(entry_id, 0));
690}
691
693void PanelView::SetOpacity(float opacity)692void PanelView::SetOpacity(float opacity)
694{693{
695 if (opacity_ == opacity)694 if (opacity_ == opacity)
696695
=== modified file 'panel/PanelView.h'
--- panel/PanelView.h 2014-02-06 22:12:59 +0000
+++ panel/PanelView.h 2014-02-11 20:09:36 +0000
@@ -65,6 +65,7 @@
6565
66 bool IsActive() const;66 bool IsActive() const;
67 bool FirstMenuShow() const;67 bool FirstMenuShow() const;
68 bool ActivateEntry(std::string const& entry_id);
6869
69 void SetOpacity(float opacity);70 void SetOpacity(float opacity);
70 void SetOpacityMaximizedToggle(bool enabled);71 void SetOpacityMaximizedToggle(bool enabled);
7172
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp 2014-01-22 01:21:20 +0000
+++ plugins/unityshell/src/unityshell.cpp 2014-02-11 20:09:36 +0000
@@ -171,6 +171,7 @@
171 , dirty_helpers_on_this_frame_(false)171 , dirty_helpers_on_this_frame_(false)
172 , back_buffer_age_(0)172 , back_buffer_age_(0)
173 , is_desktop_active_(false)173 , is_desktop_active_(false)
174 , grabber_(new GnomeKeyGrabber(screen))
174{175{
175 Timer timer;176 Timer timer;
176#ifndef USE_GLES177#ifndef USE_GLES
@@ -298,6 +299,8 @@
298 wt->Run(NULL);299 wt->Run(NULL);
299 uScreen = this;300 uScreen = this;
300301
302 optionSetShowMenuBarInitiate(boost::bind(&UnityScreen::showMenuBarInitiate, this, _1, _2, _3));
303 optionSetShowMenuBarTerminate(boost::bind(&UnityScreen::showMenuBarTerminate, this, _1, _2, _3));
301 optionSetLockScreenInitiate(boost::bind(&UnityScreen::LockScreenInitiate, this, _1, _2, _3));304 optionSetLockScreenInitiate(boost::bind(&UnityScreen::LockScreenInitiate, this, _1, _2, _3));
302 optionSetOverrideDecorationThemeNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));305 optionSetOverrideDecorationThemeNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
303 optionSetShadowXOffsetNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));306 optionSetShadowXOffsetNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
@@ -1930,6 +1933,32 @@
1930 screen->handleCompizEvent(plugin, event, option);1933 screen->handleCompizEvent(plugin, event, option);
1931}1934}
19321935
1936bool UnityScreen::showMenuBarInitiate(CompAction* action,
1937 CompAction::State state,
1938 CompOption::Vector& options)
1939{
1940 if (state & CompAction::StateInitKey)
1941 {
1942 action->setState(action->state() | CompAction::StateTermKey);
1943 panel_controller_->ShowMenuBar();
1944 }
1945
1946 return false;
1947}
1948
1949bool UnityScreen::showMenuBarTerminate(CompAction* action,
1950 CompAction::State state,
1951 CompOption::Vector& options)
1952{
1953 if (state & CompAction::StateTermKey)
1954 {
1955 action->setState(action->state() & ~CompAction::StateTermKey);
1956 panel_controller_->HideMenuBar();
1957 }
1958
1959 return false;
1960}
1961
1933bool UnityScreen::showLauncherKeyInitiate(CompAction* action,1962bool UnityScreen::showLauncherKeyInitiate(CompAction* action,
1934 CompAction::State state,1963 CompAction::State state,
1935 CompOption::Vector& options)1964 CompOption::Vector& options)
@@ -3525,7 +3554,7 @@
35253554
3526 /* Setup panel */3555 /* Setup panel */
3527 timer.Reset();3556 timer.Reset();
3528 panel_controller_ = std::make_shared<panel::Controller>(edge_barriers);3557 panel_controller_ = std::make_shared<panel::Controller>(edge_barriers, grabber_);
3529 AddChild(panel_controller_.get());3558 AddChild(panel_controller_.get());
3530 panel_controller_->SetMenuShowTimings(optionGetMenusFadein(),3559 panel_controller_->SetMenuShowTimings(optionGetMenusFadein(),
3531 optionGetMenusFadeout(),3560 optionGetMenusFadeout(),
@@ -3609,6 +3638,11 @@
3609 gestures_sub_windows_->Activate();3638 gestures_sub_windows_->Activate();
3610}3639}
36113640
3641CompAction::Vector& UnityScreen::getActions()
3642{
3643 return grabber_->getActions();
3644}
3645
3612/* Window init */3646/* Window init */
36133647
3614namespace3648namespace
36153649
=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h 2014-01-22 22:16:17 +0000
+++ plugins/unityshell/src/unityshell.h 2014-02-11 20:09:36 +0000
@@ -80,6 +80,8 @@
8080
81#include "unityshell_glow.h"81#include "unityshell_glow.h"
8282
83#include "GnomeKeyGrabber.h"
84
83namespace unity85namespace unity
84{86{
85class UnityWindow;87class UnityWindow;
@@ -106,6 +108,7 @@
106 public GLScreenInterface,108 public GLScreenInterface,
107 public BaseSwitchScreen,109 public BaseSwitchScreen,
108 public PluginClassHandler <UnityScreen, CompScreen>,110 public PluginClassHandler <UnityScreen, CompScreen>,
111 public CompAction::Container,
109 public UnityshellOptions112 public UnityshellOptions
110{113{
111public:114public:
@@ -165,6 +168,8 @@
165 void enterShowDesktopMode ();168 void enterShowDesktopMode ();
166 void leaveShowDesktopMode (CompWindow *w);169 void leaveShowDesktopMode (CompWindow *w);
167170
171 bool showMenuBarInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
172 bool showMenuBarTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options);
168 bool showLauncherKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);173 bool showLauncherKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
169 bool showLauncherKeyTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options);174 bool showLauncherKeyTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options);
170 bool showPanelFirstMenuKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);175 bool showPanelFirstMenuKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
@@ -225,6 +230,8 @@
225230
226 ui::LayoutWindow::Ptr GetSwitcherDetailLayoutWindow(Window window) const;231 ui::LayoutWindow::Ptr GetSwitcherDetailLayoutWindow(Window window) const;
227232
233 CompAction::Vector& getActions();
234
228protected:235protected:
229 std::string GetName() const;236 std::string GetName() const;
230 void AddProperties(debug::IntrospectionData&);237 void AddProperties(debug::IntrospectionData&);
@@ -399,6 +406,8 @@
399406
400 bool is_desktop_active_;407 bool is_desktop_active_;
401408
409 GnomeKeyGrabber::Ptr grabber_;
410
402 friend class UnityWindow;411 friend class UnityWindow;
403 friend class decoration::Manager;412 friend class decoration::Manager;
404};413};
405414
=== modified file 'plugins/unityshell/unityshell.xml.in'
--- plugins/unityshell/unityshell.xml.in 2014-02-03 20:42:09 +0000
+++ plugins/unityshell/unityshell.xml.in 2014-02-11 20:09:36 +0000
@@ -50,6 +50,12 @@
50 <group>50 <group>
51 <_short>General</_short>51 <_short>General</_short>
5252
53 <option name="show_menu_bar" type="key">
54 <_short>Key to show the menu bar while pressed</_short>
55 <_long>Reveals the global menu bar while pressed.</_long>
56 <default>&lt;Alt&gt;</default>
57 </option>
58
53 <option name="lock_screen" type="key">59 <option name="lock_screen" type="key">
54 <_short>Key to lock the screen.</_short>60 <_short>Key to lock the screen.</_short>
55 <_long>Pressing this key will lock the current session.</_long>61 <_long>Pressing this key will lock the current session.</_long>
@@ -57,7 +63,7 @@
57 </option>63 </option>
5864
59 <option name="show_hud" type="key">65 <option name="show_hud" type="key">
60 <_short>Key to show the HUD</_short>66 <_short>Key to show the HUD when tapped</_short>
61 <_long>A tap on this key summons the HUD.</_long>67 <_long>A tap on this key summons the HUD.</_long>
62 <default>&lt;Alt&gt;</default>68 <default>&lt;Alt&gt;</default>
63 </option>69 </option>
6470
=== added file 'tests/autopilot/unity/tests/test_gnome_key_grabber.py'
--- tests/autopilot/unity/tests/test_gnome_key_grabber.py 1970-01-01 00:00:00 +0000
+++ tests/autopilot/unity/tests/test_gnome_key_grabber.py 2014-02-11 20:09:36 +0000
@@ -0,0 +1,173 @@
1# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
2# Copyright 2014 Canonical Ltd.
3# Author: William Hua <william.hua@canonical.com>
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
9import dbus
10import glib
11import unity
12import logging
13
14from autopilot.matchers import *
15from testtools.matchers import *
16
17log = logging.getLogger(__name__)
18
19class Accelerator:
20
21 def __init__(self, accelerator=None, shortcut=None, action=0):
22 self.accelerator = accelerator
23 self.shortcut = shortcut
24 self.action = action
25
26 def __str__(self):
27 if self.action:
28 return "%u '%s'" % (self.action, self.shortcut)
29 else:
30 return "'%s'" % self.shortcut
31
32class GnomeKeyGrabberTests(unity.tests.UnityTestCase):
33 """Gnome key grabber tests"""
34
35 def setUp(self):
36 super(GnomeKeyGrabberTests, self).setUp()
37
38 dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
39
40 bus = dbus.SessionBus()
41 proxy = bus.get_object('org.gnome.Shell', '/org/gnome/Shell')
42 self.interface = dbus.Interface(proxy, 'org.gnome.Shell')
43
44 self.activatable = set()
45 self.activated = [False]
46 self.active = True
47
48 def accelerator_activated(action, device):
49 if self.active and action in self.activatable:
50 log.info('%d activated' % action)
51 self.activated[0] = True
52
53 self.signal = self.interface.connect_to_signal('AcceleratorActivated', accelerator_activated)
54
55 def tearDown(self):
56 self.active = False
57
58 super(GnomeKeyGrabberTests, self).tearDown()
59
60 def press_accelerator(self, accelerator):
61 self.activated[0] = False
62
63 # Press accelerator shortcut
64 log.info("pressing '%s'" % accelerator.shortcut)
65 self.keyboard.press_and_release(accelerator.shortcut)
66
67 loop = glib.MainLoop()
68
69 def wait():
70 loop.quit()
71 return False
72
73 glib.timeout_add_seconds(1, wait)
74 loop.run()
75
76 return self.activated[0]
77
78 def check_accelerator(self, accelerator):
79 # Check that accelerator works
80 self.assertTrue(self.press_accelerator(accelerator))
81
82 # Remove accelerator
83 log.info('ungrabbing %s' % accelerator)
84 self.assertTrue(self.interface.UngrabAccelerator(accelerator.action))
85
86 # Check that accelerator does not work
87 self.assertFalse(self.press_accelerator(accelerator))
88
89 # Try removing accelerator
90 log.info('ungrabbing %s (should fail)' % accelerator)
91 self.assertFalse(self.interface.UngrabAccelerator(accelerator.action))
92
93 def test_grab_accelerators(self):
94 accelerators = [Accelerator('<Shift><Control>x', 'Shift+Control+x'),
95 Accelerator('<Control><Alt>y', 'Control+Alt+y'),
96 Accelerator('<Shift><Alt>z', 'Shift+Alt+z')]
97
98 actions = self.interface.GrabAccelerators([(accelerator.accelerator, 0) for accelerator in accelerators])
99
100 self.activatable.clear()
101
102 for accelerator, action in zip(accelerators, actions):
103 accelerator.action = action
104 self.activatable.add(action)
105 log.info('grabbed %s' % accelerator)
106
107 def clean_up_test_grab_accelerators():
108 self.activatable.clear()
109
110 for accelerator in accelerators:
111 log.info('unconditionally ungrabbing %s' % accelerator)
112 self.interface.UngrabAccelerator(accelerator.action)
113
114 self.addCleanup(clean_up_test_grab_accelerators)
115
116 for accelerator in accelerators:
117 self.check_accelerator(accelerator)
118
119 def test_grab_accelerator(self):
120 accelerator = Accelerator('<Shift><Control><Alt>a', 'Shift+Control+Alt+a')
121 accelerator.action = self.interface.GrabAccelerator(accelerator.accelerator, 0)
122
123 self.activatable.clear()
124 self.activatable.add(accelerator.action)
125
126 log.info('grabbed %s' % accelerator)
127
128 def clean_up_test_grab_accelerator():
129 self.activatable.clear()
130 log.info('unconditionally ungrabbing %s' % accelerator)
131 self.interface.UngrabAccelerator(accelerator.action)
132
133 self.addCleanup(clean_up_test_grab_accelerator)
134
135 self.check_accelerator(accelerator)
136
137 def test_grab_same_accelerator(self):
138 accelerators = [Accelerator('<Shift><Control><Alt>b', 'Shift+Control+Alt+b') for i in xrange(3)]
139 actions = self.interface.GrabAccelerators([(accelerator.accelerator, 0) for accelerator in accelerators])
140
141 self.activatable.clear()
142
143 for accelerator, action in zip(accelerators, actions):
144 accelerator.action = action
145 self.activatable.add(action)
146 log.info('grabbed %s' % accelerator)
147
148 def clean_up_test_grab_same_accelerator():
149 self.activatable.clear()
150
151 for accelerator in accelerators:
152 log.info('unconditionally ungrabbing %s' % accelerator)
153 self.interface.UngrabAccelerator(accelerator.action)
154
155 self.addCleanup(clean_up_test_grab_same_accelerator)
156
157 for accelerator in accelerators:
158 # Check that accelerator works
159 self.assertTrue(self.press_accelerator(accelerator))
160
161 # Remove accelerator
162 log.info('ungrabbing %s' % accelerator)
163 self.assertTrue(self.interface.UngrabAccelerator(accelerator.action))
164
165 # This accelerator cannot activate any more
166 self.activatable.remove(accelerator.action)
167
168 # Add them all again for one final check
169 for accelerator in accelerators:
170 self.activatable.add(accelerator.action)
171
172 # Check that signal was not emitted
173 self.assertFalse(self.press_accelerator(accelerators[0]))
0174
=== modified file 'tests/autopilot/unity/tests/test_panel.py'
--- tests/autopilot/unity/tests/test_panel.py 2014-02-10 15:31:44 +0000
+++ tests/autopilot/unity/tests/test_panel.py 2014-02-11 20:09:36 +0000
@@ -1030,6 +1030,21 @@
1030 expected_indicator = self.panel.get_indicator_entries(include_hidden_menus=True)[0]1030 expected_indicator = self.panel.get_indicator_entries(include_hidden_menus=True)[0]
1031 self.assertThat(open_indicator.entry_id, Eventually(Equals(expected_indicator.entry_id)))1031 self.assertThat(open_indicator.entry_id, Eventually(Equals(expected_indicator.entry_id)))
10321032
1033 def test_panel_hold_show_menu_works(self):
1034 """Holding the show menu key must reveal the menu with mnemonics."""
1035 self.open_new_application_window("Text Editor")
1036 refresh_fn = lambda: len(self.panel.menus.get_entries())
1037 self.assertThat(refresh_fn, Eventually(GreaterThan(0)))
1038 self.addCleanup(self.keyboard.press_and_release, "Escape")
1039
1040 # Wait for menu to fade out first
1041 self.assertThat(self.panel.menus.get_entries()[0].visible, Eventually(Equals(0)))
1042
1043 self.keyboard.press("Alt")
1044 self.addCleanup(self.keyboard.release, "Alt")
1045 self.assertTrue(self.panel.menus.get_entries()[0].visible)
1046 self.assertThat(self.panel.menus.get_entries()[0].label, Equals("_File"))
1047
1033 def test_panel_menu_accelerators_work(self):1048 def test_panel_menu_accelerators_work(self):
1034 """Pressing a valid menu accelerator must open the correct menu item."""1049 """Pressing a valid menu accelerator must open the correct menu item."""
1035 self.open_new_application_window("Text Editor")1050 self.open_new_application_window("Text Editor")
10361051
=== modified file 'unity-shared/CMakeLists.txt'
--- unity-shared/CMakeLists.txt 2014-02-07 21:50:15 +0000
+++ unity-shared/CMakeLists.txt 2014-02-11 20:09:36 +0000
@@ -100,16 +100,24 @@
100 set (UNITY_SHARED_COMPIZ_SOURCES100 set (UNITY_SHARED_COMPIZ_SOURCES
101 CompizUtils.cpp101 CompizUtils.cpp
102 PluginAdapter.cpp102 PluginAdapter.cpp
103 GnomeKeyGrabber.cpp
103 )104 )
104105
105 find_package (PkgConfig)106 find_package (PkgConfig)
107 pkg_check_modules (COMPIZ REQUIRED compiz)
106 pkg_check_modules (COMPIZ_OPENGL REQUIRED compiz-opengl)108 pkg_check_modules (COMPIZ_OPENGL REQUIRED compiz-opengl)
107109
108 add_library (unity-shared-compiz STATIC ${UNITY_SHARED_COMPIZ_SOURCES})110 add_library (unity-shared-compiz STATIC ${UNITY_SHARED_COMPIZ_SOURCES})
109111
110 # This makes linker to include library dir in RUNPATH112 # This makes linker to include library dir in RUNPATH
113 find_library (COMPIZ_LIB compiz_core ${COMPIZ_LIBDIR})
111 find_library (COMPIZ_OPENGL_LIB opengl ${COMPIZ_OPENGL_LIBDIR})114 find_library (COMPIZ_OPENGL_LIB opengl ${COMPIZ_OPENGL_LIBDIR})
112 target_link_libraries (unity-shared-compiz ${LIBS} ${COMPIZ_OPENGL_LIB} ${COMPIZ_OPENGL_LDFLAGS})115 target_link_libraries (unity-shared-compiz
116 ${LIBS}
117 ${COMPIZ_LIB}
118 ${COMPIZ_LDFLAGS}
119 ${COMPIZ_OPENGL_LIB}
120 ${COMPIZ_OPENGL_LDFLAGS})
113 add_dependencies (unity-shared-compiz unity-shared)121 add_dependencies (unity-shared-compiz unity-shared)
114122
115 # bamf application manager123 # bamf application manager
116124
=== added file 'unity-shared/GnomeKeyGrabber.cpp'
--- unity-shared/GnomeKeyGrabber.cpp 1970-01-01 00:00:00 +0000
+++ unity-shared/GnomeKeyGrabber.cpp 2014-02-11 20:09:36 +0000
@@ -0,0 +1,268 @@
1// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
2/*
3 * Copyright (C) 2013 Canonical Ltd
4 *
5 * This program is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation.
8 *
9 * This program is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details.
13 *
14 * You should have received a copy of the GNU General Public License
15 * along with this program. If not, see <http://www.gnu.org/licenses/>.
16 *
17 * Authored by: William Hua <william.hua@canonical.com>
18 */
19
20#include "GnomeKeyGrabberImpl.h"
21
22#include <NuxCore/Logger.h>
23
24namespace unity
25{
26DECLARE_LOGGER(logger, "unity.gnome");
27
28// Private implementation
29namespace shell
30{
31std::string const DBUS_NAME = "org.gnome.Shell";
32std::string const DBUS_INTERFACE = "org.gnome.Shell";
33std::string const DBUS_OBJECT_PATH = "/org/gnome/Shell";
34std::string const INTROSPECTION_XML =
35R"(<node>
36 <interface name='org.gnome.Shell'>
37 <method name='GrabAccelerators'>
38 <arg type='a(su)' direction='in' name='accelerators'/>
39 <arg type='au' direction='out' name='actions'/>
40 </method>
41 <method name='GrabAccelerator'>
42 <arg type='s' direction='in' name='accelerator'/>
43 <arg type='u' direction='in' name='flags'/>
44 <arg type='u' direction='out' name='action'/>
45 </method>
46 <method name='UngrabAccelerator'>
47 <arg type='u' direction='in' name='action'/>
48 <arg type='b' direction='out' name='success'/>
49 </method>
50 <signal name='AcceleratorActivated'>
51 <arg type='u' name='action'/>
52 <arg type='u' name='device'/>
53 </signal>
54 </interface>
55</node>)";
56}
57
58namespace testing
59{
60std::string const DBUS_NAME = "com.canonical.Unity.Test.GnomeKeyGrabber";
61}
62
63GnomeKeyGrabber::Impl::Impl(CompScreen* screen, bool test_mode)
64 : test_mode_(test_mode)
65 , shell_server_(test_mode_ ? testing::DBUS_NAME : shell::DBUS_NAME)
66 , screen_(screen)
67 , current_action_id_(0)
68{
69 shell_server_.AddObjects(shell::INTROSPECTION_XML, shell::DBUS_OBJECT_PATH);
70 shell_object_ = shell_server_.GetObject(shell::DBUS_INTERFACE);
71 shell_object_->SetMethodsCallsHandler(sigc::mem_fun(this, &Impl::onShellMethodCall));
72}
73
74GnomeKeyGrabber::Impl::~Impl()
75{
76 if (screen_)
77 {
78 for (auto& action : actions_)
79 screen_->removeAction(&action);
80 }
81}
82
83unsigned int GnomeKeyGrabber::Impl::addAction(CompAction const& action, bool addressable)
84{
85 ++current_action_id_;
86 actions_.push_back(action);
87 action_ids_.push_back(current_action_id_);
88
89 if (addressable)
90 {
91 action_ids_by_action_[&action] = current_action_id_;
92 actions_by_action_id_[current_action_id_] = &action;
93 }
94
95 if (screen_)
96 screen_->addAction(&actions_.back());
97
98 return current_action_id_;
99}
100
101bool GnomeKeyGrabber::Impl::removeAction(CompAction const& action)
102{
103 auto i = action_ids_by_action_.find(&action);
104 return i != action_ids_by_action_.end() && removeAction(i->second);
105}
106
107bool GnomeKeyGrabber::Impl::removeAction(unsigned int action_id)
108{
109 auto i = std::find(action_ids_.begin(), action_ids_.end(), action_id);
110
111 if (i != action_ids_.end())
112 {
113 auto j = actions_.begin() + (i - action_ids_.begin());
114 auto k = actions_by_action_id_.find(action_id);
115
116 if (screen_)
117 screen_->removeAction(&(*j));
118
119 if (k != actions_by_action_id_.end())
120 {
121 action_ids_by_action_.erase(k->second);
122 actions_by_action_id_.erase(k);
123 }
124
125 action_ids_.erase(i);
126 actions_.erase(j);
127 return true;
128 }
129
130 return false;
131}
132
133GVariant* GnomeKeyGrabber::Impl::onShellMethodCall(std::string const& method, GVariant* parameters)
134{
135 LOG_DEBUG(logger) << "Called method '" << method << "'";
136
137 if (method == "GrabAccelerators")
138 {
139 if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(a(su))")))
140 {
141 GVariant* variant;
142 GVariantBuilder builder;
143 GVariantIter* iterator;
144 gchar const* accelerator;
145 guint flags;
146
147 g_variant_builder_init(&builder, G_VARIANT_TYPE("au"));
148 g_variant_get(parameters, "(a(su))", &iterator);
149
150 while (g_variant_iter_next(iterator, "(&su)", &accelerator, &flags))
151 g_variant_builder_add(&builder, "u", grabAccelerator(accelerator, flags));
152
153 g_variant_iter_free(iterator);
154 variant = g_variant_builder_end(&builder);
155 return g_variant_new_tuple(&variant, 1);
156 }
157 else
158 LOG_WARN(logger) << "Expected arguments of type (a(su))";
159 }
160 else if (method == "GrabAccelerator")
161 {
162 if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(su)")))
163 {
164 GVariant* variant;
165 gchar const* accelerator;
166 guint flags;
167
168 g_variant_get(parameters, "(&su)", &accelerator, &flags);
169 variant = g_variant_new_uint32(grabAccelerator(accelerator, flags));
170 return g_variant_new_tuple(&variant, 1);
171 }
172 else
173 LOG_WARN(logger) << "Expected arguments of type (su)";
174 }
175 else if (method == "UngrabAccelerator")
176 {
177 if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(u)")))
178 {
179 GVariant* variant;
180 guint action;
181
182 g_variant_get(parameters, "(u)", &action);
183 variant = g_variant_new_boolean(removeAction(action));
184 return g_variant_new_tuple(&variant, 1);
185 }
186 else
187 LOG_WARN(logger) << "Expected arguments of type (u)";
188 }
189
190 return nullptr;
191}
192
193unsigned int GnomeKeyGrabber::Impl::grabAccelerator(char const* accelerator, unsigned int flags)
194{
195 CompAction action;
196 action.keyFromString(accelerator);
197
198 if (!isActionPostponed(action))
199 {
200 action.setState(CompAction::StateInitKey);
201 action.setInitiate([this](CompAction* action, CompAction::State state, CompOption::Vector& options) {
202 activateAction(action, 0);
203 return true;
204 });
205 }
206 else
207 {
208 action.setState(CompAction::StateInitKey | CompAction::StateTermKey);
209 action.setTerminate([this](CompAction* action, CompAction::State state, CompOption::Vector& options) {
210 if (state & CompAction::StateTermTapped)
211 {
212 activateAction(action, 0);
213 return true;
214 }
215
216 return false;
217 });
218 }
219
220 return addAction(action, false);
221}
222
223void GnomeKeyGrabber::Impl::activateAction(CompAction const* action, unsigned int device) const
224{
225 ptrdiff_t i = action - &actions_.front();
226
227 if (0 <= i && i < static_cast<ptrdiff_t>(action_ids_.size()))
228 shell_object_->EmitSignal("AcceleratorActivated", g_variant_new("(uu)", action_ids_[i], device));
229}
230
231bool GnomeKeyGrabber::Impl::isActionPostponed(CompAction const& action) const
232{
233 int keycode = action.key().keycode();
234 return keycode == 0 || modHandler->keycodeToModifiers(keycode) != 0;
235}
236
237// Public implementation
238
239GnomeKeyGrabber::GnomeKeyGrabber(CompScreen* screen)
240 : impl_(new Impl(screen))
241{
242}
243
244GnomeKeyGrabber::GnomeKeyGrabber(CompScreen* screen, TestMode const& dummy)
245 : impl_(new Impl(screen, true))
246{
247}
248
249GnomeKeyGrabber::~GnomeKeyGrabber()
250{
251}
252
253CompAction::Vector& GnomeKeyGrabber::getActions()
254{
255 return impl_->actions_;
256}
257
258void GnomeKeyGrabber::addAction(CompAction const& action)
259{
260 impl_->addAction(action);
261}
262
263void GnomeKeyGrabber::removeAction(CompAction const& action)
264{
265 impl_->removeAction(action);
266}
267
268} // namespace unity
0269
=== added file 'unity-shared/GnomeKeyGrabber.h'
--- unity-shared/GnomeKeyGrabber.h 1970-01-01 00:00:00 +0000
+++ unity-shared/GnomeKeyGrabber.h 2014-02-11 20:09:36 +0000
@@ -0,0 +1,54 @@
1// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
2/*
3* Copyright (C) 2013 Canonical Ltd
4*
5* This program is free software: you can redistribute it and/or modify
6* it under the terms of the GNU General Public License version 3 as
7* published by the Free Software Foundation.
8*
9* This program is distributed in the hope that it will be useful,
10* but WITHOUT ANY WARRANTY; without even the implied warranty of
11* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12* GNU General Public License for more details.
13*
14* You should have received a copy of the GNU General Public License
15* along with this program. If not, see <http://www.gnu.org/licenses/>.
16*
17* Authored by: William Hua <william.hua@canonical.com>
18*/
19
20#ifndef __GNOME_KEY_GRABBER_H__
21#define __GNOME_KEY_GRABBER_H__
22
23#include <core/core.h>
24
25namespace unity
26{
27
28class GnomeKeyGrabber
29{
30public:
31
32 typedef std::shared_ptr<GnomeKeyGrabber> Ptr;
33
34 explicit GnomeKeyGrabber(CompScreen* screen);
35 virtual ~GnomeKeyGrabber();
36
37 CompAction::Vector& getActions();
38 void addAction(CompAction const& action);
39 void removeAction(CompAction const& action);
40
41protected:
42
43 struct TestMode {};
44 GnomeKeyGrabber(CompScreen* screen, TestMode const& dummy);
45
46private:
47
48 struct Impl;
49 std::unique_ptr<Impl> impl_;
50};
51
52} // namespace unity
53
54#endif // __GNOME_KEY_GRABBER_H__
055
=== added file 'unity-shared/GnomeKeyGrabberImpl.h'
--- unity-shared/GnomeKeyGrabberImpl.h 1970-01-01 00:00:00 +0000
+++ unity-shared/GnomeKeyGrabberImpl.h 2014-02-11 20:09:36 +0000
@@ -0,0 +1,64 @@
1// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
2/*
3* Copyright (C) 2013 Canonical Ltd
4*
5* This program is free software: you can redistribute it and/or modify
6* it under the terms of the GNU General Public License version 3 as
7* published by the Free Software Foundation.
8*
9* This program is distributed in the hope that it will be useful,
10* but WITHOUT ANY WARRANTY; without even the implied warranty of
11* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12* GNU General Public License for more details.
13*
14* You should have received a copy of the GNU General Public License
15* along with this program. If not, see <http://www.gnu.org/licenses/>.
16*
17* Authored by: William Hua <william.hua@canonical.com>
18*/
19
20#ifndef __GNOME_KEY_GRABBER_IMPL_H__
21#define __GNOME_KEY_GRABBER_IMPL_H__
22
23#include "GnomeKeyGrabber.h"
24
25#include <UnityCore/GLibDBusProxy.h>
26#include <UnityCore/GLibDBusServer.h>
27
28#include <unordered_map>
29
30namespace unity
31{
32
33struct GnomeKeyGrabber::Impl
34{
35 bool test_mode_;
36
37 glib::DBusServer shell_server_;
38 glib::DBusObject::Ptr shell_object_;
39
40 CompScreen* screen_;
41 CompAction::Vector actions_;
42 std::vector<unsigned int> action_ids_;
43 unsigned int current_action_id_;
44
45 std::unordered_map<CompAction const*, unsigned int> action_ids_by_action_;
46 std::unordered_map<unsigned int, CompAction const*> actions_by_action_id_;
47
48 explicit Impl(CompScreen* screen, bool test_mode = false);
49 ~Impl();
50
51 unsigned int addAction(CompAction const& action, bool addressable = true);
52 bool removeAction(CompAction const& action);
53 bool removeAction(unsigned int action_id);
54
55 GVariant* onShellMethodCall(std::string const& method, GVariant* parameters);
56 unsigned int grabAccelerator(char const* accelerator, unsigned int flags);
57 void activateAction(CompAction const* action, unsigned int device) const;
58
59 bool isActionPostponed(CompAction const& action) const;
60};
61
62} // namespace unity
63
64#endif // __GNOME_KEY_GRABBER_IMPL_H__
065
=== modified file 'unity-standalone/CMakeLists.txt'
--- unity-standalone/CMakeLists.txt 2012-12-19 14:39:56 +0000
+++ unity-standalone/CMakeLists.txt 2014-02-11 20:09:36 +0000
@@ -27,4 +27,5 @@
27 panel-lib27 panel-lib
28 unity-shared28 unity-shared
29 unity-shared-bamf29 unity-shared-bamf
30 unity-shared-compiz
30 unity-shared-standalone)31 unity-shared-standalone)