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
1=== modified file 'panel/PanelController.cpp'
2--- panel/PanelController.cpp 2014-01-25 13:39:49 +0000
3+++ panel/PanelController.cpp 2014-02-11 20:09:36 +0000
4@@ -39,9 +39,15 @@
5 class Controller::Impl
6 {
7 public:
8- Impl(ui::EdgeBarrierController::Ptr const& edge_barriers);
9+ Impl(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber);
10 ~Impl();
11
12+ void GrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator);
13+ void UngrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator);
14+ void GrabEntryMnemonics(indicator::Entry::Ptr const& entry);
15+ void UngrabEntryMnemonics(std::string const& entry_id);
16+
17+ void SetMenuBarVisible(bool visible);
18 void FirstMenuShow();
19 void QueueRedraw();
20
21@@ -64,6 +70,7 @@
22 PanelView* ViewForWindow(BaseWindowPtr const& window) const;
23
24 ui::EdgeBarrierController::Ptr edge_barriers_;
25+ GnomeKeyGrabber::Ptr grabber_;
26 PanelVector panels_;
27 std::vector<nux::Geometry> panel_geometries_;
28 std::vector<Window> tray_xids_;
29@@ -75,11 +82,15 @@
30 int menus_discovery_fadein_;
31 int menus_discovery_fadeout_;
32 indicator::DBusIndicators::Ptr dbus_indicators_;
33+ std::unordered_map<std::string, std::shared_ptr<CompAction>> entry_actions_;
34+ connection::Manager dbus_indicators_connections_;
35+ std::unordered_map<indicator::Indicator::Ptr, connection::Manager> indicator_connections_;
36 };
37
38
39-Controller::Impl::Impl(ui::EdgeBarrierController::Ptr const& edge_barriers)
40+Controller::Impl::Impl(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber)
41 : edge_barriers_(edge_barriers)
42+ , grabber_(grabber)
43 , opacity_(1.0f)
44 , opacity_maximized_toggle_(false)
45 , menus_fadein_(0)
46@@ -88,10 +99,28 @@
47 , menus_discovery_fadein_(0)
48 , menus_discovery_fadeout_(0)
49 , dbus_indicators_(std::make_shared<indicator::DBusIndicators>())
50-{}
51+{
52+ if (grabber_)
53+ {
54+ for (auto const& indicator : dbus_indicators_->GetIndicators())
55+ GrabIndicatorMnemonics(indicator);
56+
57+ auto& connections = dbus_indicators_connections_;
58+ connections.Add(dbus_indicators_->on_object_added.connect(sigc::mem_fun(this, &Impl::GrabIndicatorMnemonics)));
59+ connections.Add(dbus_indicators_->on_object_removed.connect(sigc::mem_fun(this, &Impl::UngrabIndicatorMnemonics)));
60+ }
61+}
62
63 Controller::Impl::~Impl()
64 {
65+ if (grabber_)
66+ {
67+ dbus_indicators_connections_.Clear();
68+
69+ for (auto const& indicator : dbus_indicators_->GetIndicators())
70+ UngrabIndicatorMnemonics(indicator);
71+ }
72+
73 // Since the panels are in a window which adds a reference to the
74 // panel, we need to make sure the base windows are unreferenced
75 // otherwise the pnales never die.
76@@ -102,6 +131,70 @@
77 }
78 }
79
80+void Controller::Impl::GrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator)
81+{
82+ if (indicator->IsAppmenu())
83+ {
84+ for (auto const& entry : indicator->GetEntries())
85+ GrabEntryMnemonics(entry);
86+
87+ auto& connections = indicator_connections_[indicator];
88+ connections.Add(indicator->on_entry_added.connect(sigc::mem_fun(this, &Impl::GrabEntryMnemonics)));
89+ connections.Add(indicator->on_entry_removed.connect(sigc::mem_fun(this, &Impl::UngrabEntryMnemonics)));
90+ }
91+}
92+
93+void Controller::Impl::UngrabIndicatorMnemonics(indicator::Indicator::Ptr const& indicator)
94+{
95+ if (indicator->IsAppmenu())
96+ {
97+ indicator_connections_.erase(indicator);
98+
99+ for (auto const& entry : indicator->GetEntries())
100+ UngrabEntryMnemonics(entry->id());
101+ }
102+}
103+
104+void Controller::Impl::GrabEntryMnemonics(indicator::Entry::Ptr const& entry)
105+{
106+ gunichar mnemonic;
107+
108+ if (pango_parse_markup(entry->label().c_str(), -1, '_', nullptr, nullptr, &mnemonic, nullptr) && mnemonic)
109+ {
110+ auto key = gdk_keyval_to_lower(gdk_unicode_to_keyval(mnemonic));
111+ glib::String accelerator(gtk_accelerator_name(key, GDK_MOD1_MASK));
112+ auto action = std::make_shared<CompAction>();
113+ auto id = entry->id();
114+
115+ action->keyFromString(accelerator);
116+ action->setState(CompAction::StateInitKey);
117+ action->setInitiate([this, id](CompAction* action, CompAction::State state, CompOption::Vector& options)
118+ {
119+ for (auto const& panel : panels_)
120+ {
121+ if (panel->ActivateEntry(id))
122+ return true;
123+ }
124+
125+ return false;
126+ });
127+
128+ entry_actions_[id] = action;
129+ grabber_->addAction(*action);
130+ }
131+}
132+
133+void Controller::Impl::UngrabEntryMnemonics(std::string const& entry_id)
134+{
135+ auto i = entry_actions_.find(entry_id);
136+
137+ if (i != entry_actions_.end())
138+ {
139+ grabber_->removeAction(*i->second);
140+ entry_actions_.erase(i);
141+ }
142+}
143+
144 void Controller::Impl::UpdatePanelGeometries()
145 {
146 panel_geometries_.reserve(panels_.size());
147@@ -112,6 +205,20 @@
148 }
149 }
150
151+void Controller::Impl::SetMenuBarVisible(bool visible)
152+{
153+ for (auto const& indicator : dbus_indicators_->GetIndicators())
154+ {
155+ if (indicator->IsAppmenu())
156+ {
157+ for (auto const& entry : indicator->GetEntries())
158+ entry->set_show_now(visible);
159+
160+ break;
161+ }
162+ }
163+}
164+
165 void Controller::Impl::FirstMenuShow()
166 {
167 for (auto const& panel: panels_)
168@@ -279,9 +386,9 @@
169 return opacity_;
170 }
171
172-Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers)
173+Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers, GnomeKeyGrabber::Ptr const& grabber)
174 : launcher_width(64)
175- , pimpl(new Impl(edge_barriers))
176+ , pimpl(new Impl(edge_barriers, grabber))
177 {
178 UScreen* screen = UScreen::GetDefault();
179 screen->changed.connect(sigc::mem_fun(this, &Controller::OnScreenChanged));
180@@ -293,11 +400,26 @@
181 });
182 }
183
184+Controller::Controller(ui::EdgeBarrierController::Ptr const& edge_barriers)
185+ : Controller(edge_barriers, GnomeKeyGrabber::Ptr())
186+{
187+}
188+
189 Controller::~Controller()
190 {
191 delete pimpl;
192 }
193
194+void Controller::ShowMenuBar()
195+{
196+ pimpl->SetMenuBarVisible(true);
197+}
198+
199+void Controller::HideMenuBar()
200+{
201+ pimpl->SetMenuBarVisible(false);
202+}
203+
204 void Controller::FirstMenuShow()
205 {
206 pimpl->FirstMenuShow();
207
208=== modified file 'panel/PanelController.h'
209--- panel/PanelController.h 2014-01-25 13:39:49 +0000
210+++ panel/PanelController.h 2014-02-11 20:09:36 +0000
211@@ -24,7 +24,9 @@
212 #include <Nux/Nux.h>
213
214 #include "launcher/EdgeBarrierController.h"
215+#include "unity-shared/GnomeKeyGrabber.h"
216 #include "unity-shared/Introspectable.h"
217+
218 namespace unity
219 {
220 namespace panel
221@@ -37,9 +39,12 @@
222 typedef std::shared_ptr<Controller> Ptr;
223 typedef std::vector<nux::ObjectPtr<PanelView>> PanelVector;
224
225+ Controller(ui::EdgeBarrierController::Ptr const& barrier_controller, GnomeKeyGrabber::Ptr const& grabber);
226 Controller(ui::EdgeBarrierController::Ptr const& barrier_controller);
227 ~Controller();
228
229+ void ShowMenuBar();
230+ void HideMenuBar();
231 void FirstMenuShow();
232 void QueueRedraw();
233
234
235=== modified file 'panel/PanelView.cpp'
236--- panel/PanelView.cpp 2014-02-06 22:12:59 +0000
237+++ panel/PanelView.cpp 2014-02-11 20:09:36 +0000
238@@ -592,13 +592,7 @@
239
240 void PanelView::OnEntryActivateRequest(std::string const& entry_id)
241 {
242- if (!IsActive())
243- return;
244-
245- bool ret;
246-
247- ret = menu_view_->ActivateEntry(entry_id, 0);
248- if (!ret) indicators_->ActivateEntry(entry_id, 0);
249+ ActivateEntry(entry_id);
250 }
251
252 bool PanelView::TrackMenuPointer()
253@@ -690,6 +684,11 @@
254 return ret;
255 }
256
257+bool PanelView::ActivateEntry(std::string const& entry_id)
258+{
259+ return IsActive() && (menu_view_->ActivateEntry(entry_id, 0) || indicators_->ActivateEntry(entry_id, 0));
260+}
261+
262 void PanelView::SetOpacity(float opacity)
263 {
264 if (opacity_ == opacity)
265
266=== modified file 'panel/PanelView.h'
267--- panel/PanelView.h 2014-02-06 22:12:59 +0000
268+++ panel/PanelView.h 2014-02-11 20:09:36 +0000
269@@ -65,6 +65,7 @@
270
271 bool IsActive() const;
272 bool FirstMenuShow() const;
273+ bool ActivateEntry(std::string const& entry_id);
274
275 void SetOpacity(float opacity);
276 void SetOpacityMaximizedToggle(bool enabled);
277
278=== modified file 'plugins/unityshell/src/unityshell.cpp'
279--- plugins/unityshell/src/unityshell.cpp 2014-01-22 01:21:20 +0000
280+++ plugins/unityshell/src/unityshell.cpp 2014-02-11 20:09:36 +0000
281@@ -171,6 +171,7 @@
282 , dirty_helpers_on_this_frame_(false)
283 , back_buffer_age_(0)
284 , is_desktop_active_(false)
285+ , grabber_(new GnomeKeyGrabber(screen))
286 {
287 Timer timer;
288 #ifndef USE_GLES
289@@ -298,6 +299,8 @@
290 wt->Run(NULL);
291 uScreen = this;
292
293+ optionSetShowMenuBarInitiate(boost::bind(&UnityScreen::showMenuBarInitiate, this, _1, _2, _3));
294+ optionSetShowMenuBarTerminate(boost::bind(&UnityScreen::showMenuBarTerminate, this, _1, _2, _3));
295 optionSetLockScreenInitiate(boost::bind(&UnityScreen::LockScreenInitiate, this, _1, _2, _3));
296 optionSetOverrideDecorationThemeNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
297 optionSetShadowXOffsetNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
298@@ -1930,6 +1933,32 @@
299 screen->handleCompizEvent(plugin, event, option);
300 }
301
302+bool UnityScreen::showMenuBarInitiate(CompAction* action,
303+ CompAction::State state,
304+ CompOption::Vector& options)
305+{
306+ if (state & CompAction::StateInitKey)
307+ {
308+ action->setState(action->state() | CompAction::StateTermKey);
309+ panel_controller_->ShowMenuBar();
310+ }
311+
312+ return false;
313+}
314+
315+bool UnityScreen::showMenuBarTerminate(CompAction* action,
316+ CompAction::State state,
317+ CompOption::Vector& options)
318+{
319+ if (state & CompAction::StateTermKey)
320+ {
321+ action->setState(action->state() & ~CompAction::StateTermKey);
322+ panel_controller_->HideMenuBar();
323+ }
324+
325+ return false;
326+}
327+
328 bool UnityScreen::showLauncherKeyInitiate(CompAction* action,
329 CompAction::State state,
330 CompOption::Vector& options)
331@@ -3525,7 +3554,7 @@
332
333 /* Setup panel */
334 timer.Reset();
335- panel_controller_ = std::make_shared<panel::Controller>(edge_barriers);
336+ panel_controller_ = std::make_shared<panel::Controller>(edge_barriers, grabber_);
337 AddChild(panel_controller_.get());
338 panel_controller_->SetMenuShowTimings(optionGetMenusFadein(),
339 optionGetMenusFadeout(),
340@@ -3609,6 +3638,11 @@
341 gestures_sub_windows_->Activate();
342 }
343
344+CompAction::Vector& UnityScreen::getActions()
345+{
346+ return grabber_->getActions();
347+}
348+
349 /* Window init */
350
351 namespace
352
353=== modified file 'plugins/unityshell/src/unityshell.h'
354--- plugins/unityshell/src/unityshell.h 2014-01-22 22:16:17 +0000
355+++ plugins/unityshell/src/unityshell.h 2014-02-11 20:09:36 +0000
356@@ -80,6 +80,8 @@
357
358 #include "unityshell_glow.h"
359
360+#include "GnomeKeyGrabber.h"
361+
362 namespace unity
363 {
364 class UnityWindow;
365@@ -106,6 +108,7 @@
366 public GLScreenInterface,
367 public BaseSwitchScreen,
368 public PluginClassHandler <UnityScreen, CompScreen>,
369+ public CompAction::Container,
370 public UnityshellOptions
371 {
372 public:
373@@ -165,6 +168,8 @@
374 void enterShowDesktopMode ();
375 void leaveShowDesktopMode (CompWindow *w);
376
377+ bool showMenuBarInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
378+ bool showMenuBarTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options);
379 bool showLauncherKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
380 bool showLauncherKeyTerminate(CompAction* action, CompAction::State state, CompOption::Vector& options);
381 bool showPanelFirstMenuKeyInitiate(CompAction* action, CompAction::State state, CompOption::Vector& options);
382@@ -225,6 +230,8 @@
383
384 ui::LayoutWindow::Ptr GetSwitcherDetailLayoutWindow(Window window) const;
385
386+ CompAction::Vector& getActions();
387+
388 protected:
389 std::string GetName() const;
390 void AddProperties(debug::IntrospectionData&);
391@@ -399,6 +406,8 @@
392
393 bool is_desktop_active_;
394
395+ GnomeKeyGrabber::Ptr grabber_;
396+
397 friend class UnityWindow;
398 friend class decoration::Manager;
399 };
400
401=== modified file 'plugins/unityshell/unityshell.xml.in'
402--- plugins/unityshell/unityshell.xml.in 2014-02-03 20:42:09 +0000
403+++ plugins/unityshell/unityshell.xml.in 2014-02-11 20:09:36 +0000
404@@ -50,6 +50,12 @@
405 <group>
406 <_short>General</_short>
407
408+ <option name="show_menu_bar" type="key">
409+ <_short>Key to show the menu bar while pressed</_short>
410+ <_long>Reveals the global menu bar while pressed.</_long>
411+ <default>&lt;Alt&gt;</default>
412+ </option>
413+
414 <option name="lock_screen" type="key">
415 <_short>Key to lock the screen.</_short>
416 <_long>Pressing this key will lock the current session.</_long>
417@@ -57,7 +63,7 @@
418 </option>
419
420 <option name="show_hud" type="key">
421- <_short>Key to show the HUD</_short>
422+ <_short>Key to show the HUD when tapped</_short>
423 <_long>A tap on this key summons the HUD.</_long>
424 <default>&lt;Alt&gt;</default>
425 </option>
426
427=== added file 'tests/autopilot/unity/tests/test_gnome_key_grabber.py'
428--- tests/autopilot/unity/tests/test_gnome_key_grabber.py 1970-01-01 00:00:00 +0000
429+++ tests/autopilot/unity/tests/test_gnome_key_grabber.py 2014-02-11 20:09:36 +0000
430@@ -0,0 +1,173 @@
431+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
432+# Copyright 2014 Canonical Ltd.
433+# Author: William Hua <william.hua@canonical.com>
434+#
435+# This program is free software: you can redistribute it and/or modify it
436+# under the terms of the GNU General Public License version 3, as published
437+# by the Free Software Foundation.
438+
439+import dbus
440+import glib
441+import unity
442+import logging
443+
444+from autopilot.matchers import *
445+from testtools.matchers import *
446+
447+log = logging.getLogger(__name__)
448+
449+class Accelerator:
450+
451+ def __init__(self, accelerator=None, shortcut=None, action=0):
452+ self.accelerator = accelerator
453+ self.shortcut = shortcut
454+ self.action = action
455+
456+ def __str__(self):
457+ if self.action:
458+ return "%u '%s'" % (self.action, self.shortcut)
459+ else:
460+ return "'%s'" % self.shortcut
461+
462+class GnomeKeyGrabberTests(unity.tests.UnityTestCase):
463+ """Gnome key grabber tests"""
464+
465+ def setUp(self):
466+ super(GnomeKeyGrabberTests, self).setUp()
467+
468+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
469+
470+ bus = dbus.SessionBus()
471+ proxy = bus.get_object('org.gnome.Shell', '/org/gnome/Shell')
472+ self.interface = dbus.Interface(proxy, 'org.gnome.Shell')
473+
474+ self.activatable = set()
475+ self.activated = [False]
476+ self.active = True
477+
478+ def accelerator_activated(action, device):
479+ if self.active and action in self.activatable:
480+ log.info('%d activated' % action)
481+ self.activated[0] = True
482+
483+ self.signal = self.interface.connect_to_signal('AcceleratorActivated', accelerator_activated)
484+
485+ def tearDown(self):
486+ self.active = False
487+
488+ super(GnomeKeyGrabberTests, self).tearDown()
489+
490+ def press_accelerator(self, accelerator):
491+ self.activated[0] = False
492+
493+ # Press accelerator shortcut
494+ log.info("pressing '%s'" % accelerator.shortcut)
495+ self.keyboard.press_and_release(accelerator.shortcut)
496+
497+ loop = glib.MainLoop()
498+
499+ def wait():
500+ loop.quit()
501+ return False
502+
503+ glib.timeout_add_seconds(1, wait)
504+ loop.run()
505+
506+ return self.activated[0]
507+
508+ def check_accelerator(self, accelerator):
509+ # Check that accelerator works
510+ self.assertTrue(self.press_accelerator(accelerator))
511+
512+ # Remove accelerator
513+ log.info('ungrabbing %s' % accelerator)
514+ self.assertTrue(self.interface.UngrabAccelerator(accelerator.action))
515+
516+ # Check that accelerator does not work
517+ self.assertFalse(self.press_accelerator(accelerator))
518+
519+ # Try removing accelerator
520+ log.info('ungrabbing %s (should fail)' % accelerator)
521+ self.assertFalse(self.interface.UngrabAccelerator(accelerator.action))
522+
523+ def test_grab_accelerators(self):
524+ accelerators = [Accelerator('<Shift><Control>x', 'Shift+Control+x'),
525+ Accelerator('<Control><Alt>y', 'Control+Alt+y'),
526+ Accelerator('<Shift><Alt>z', 'Shift+Alt+z')]
527+
528+ actions = self.interface.GrabAccelerators([(accelerator.accelerator, 0) for accelerator in accelerators])
529+
530+ self.activatable.clear()
531+
532+ for accelerator, action in zip(accelerators, actions):
533+ accelerator.action = action
534+ self.activatable.add(action)
535+ log.info('grabbed %s' % accelerator)
536+
537+ def clean_up_test_grab_accelerators():
538+ self.activatable.clear()
539+
540+ for accelerator in accelerators:
541+ log.info('unconditionally ungrabbing %s' % accelerator)
542+ self.interface.UngrabAccelerator(accelerator.action)
543+
544+ self.addCleanup(clean_up_test_grab_accelerators)
545+
546+ for accelerator in accelerators:
547+ self.check_accelerator(accelerator)
548+
549+ def test_grab_accelerator(self):
550+ accelerator = Accelerator('<Shift><Control><Alt>a', 'Shift+Control+Alt+a')
551+ accelerator.action = self.interface.GrabAccelerator(accelerator.accelerator, 0)
552+
553+ self.activatable.clear()
554+ self.activatable.add(accelerator.action)
555+
556+ log.info('grabbed %s' % accelerator)
557+
558+ def clean_up_test_grab_accelerator():
559+ self.activatable.clear()
560+ log.info('unconditionally ungrabbing %s' % accelerator)
561+ self.interface.UngrabAccelerator(accelerator.action)
562+
563+ self.addCleanup(clean_up_test_grab_accelerator)
564+
565+ self.check_accelerator(accelerator)
566+
567+ def test_grab_same_accelerator(self):
568+ accelerators = [Accelerator('<Shift><Control><Alt>b', 'Shift+Control+Alt+b') for i in xrange(3)]
569+ actions = self.interface.GrabAccelerators([(accelerator.accelerator, 0) for accelerator in accelerators])
570+
571+ self.activatable.clear()
572+
573+ for accelerator, action in zip(accelerators, actions):
574+ accelerator.action = action
575+ self.activatable.add(action)
576+ log.info('grabbed %s' % accelerator)
577+
578+ def clean_up_test_grab_same_accelerator():
579+ self.activatable.clear()
580+
581+ for accelerator in accelerators:
582+ log.info('unconditionally ungrabbing %s' % accelerator)
583+ self.interface.UngrabAccelerator(accelerator.action)
584+
585+ self.addCleanup(clean_up_test_grab_same_accelerator)
586+
587+ for accelerator in accelerators:
588+ # Check that accelerator works
589+ self.assertTrue(self.press_accelerator(accelerator))
590+
591+ # Remove accelerator
592+ log.info('ungrabbing %s' % accelerator)
593+ self.assertTrue(self.interface.UngrabAccelerator(accelerator.action))
594+
595+ # This accelerator cannot activate any more
596+ self.activatable.remove(accelerator.action)
597+
598+ # Add them all again for one final check
599+ for accelerator in accelerators:
600+ self.activatable.add(accelerator.action)
601+
602+ # Check that signal was not emitted
603+ self.assertFalse(self.press_accelerator(accelerators[0]))
604
605=== modified file 'tests/autopilot/unity/tests/test_panel.py'
606--- tests/autopilot/unity/tests/test_panel.py 2014-02-10 15:31:44 +0000
607+++ tests/autopilot/unity/tests/test_panel.py 2014-02-11 20:09:36 +0000
608@@ -1030,6 +1030,21 @@
609 expected_indicator = self.panel.get_indicator_entries(include_hidden_menus=True)[0]
610 self.assertThat(open_indicator.entry_id, Eventually(Equals(expected_indicator.entry_id)))
611
612+ def test_panel_hold_show_menu_works(self):
613+ """Holding the show menu key must reveal the menu with mnemonics."""
614+ self.open_new_application_window("Text Editor")
615+ refresh_fn = lambda: len(self.panel.menus.get_entries())
616+ self.assertThat(refresh_fn, Eventually(GreaterThan(0)))
617+ self.addCleanup(self.keyboard.press_and_release, "Escape")
618+
619+ # Wait for menu to fade out first
620+ self.assertThat(self.panel.menus.get_entries()[0].visible, Eventually(Equals(0)))
621+
622+ self.keyboard.press("Alt")
623+ self.addCleanup(self.keyboard.release, "Alt")
624+ self.assertTrue(self.panel.menus.get_entries()[0].visible)
625+ self.assertThat(self.panel.menus.get_entries()[0].label, Equals("_File"))
626+
627 def test_panel_menu_accelerators_work(self):
628 """Pressing a valid menu accelerator must open the correct menu item."""
629 self.open_new_application_window("Text Editor")
630
631=== modified file 'unity-shared/CMakeLists.txt'
632--- unity-shared/CMakeLists.txt 2014-02-07 21:50:15 +0000
633+++ unity-shared/CMakeLists.txt 2014-02-11 20:09:36 +0000
634@@ -100,16 +100,24 @@
635 set (UNITY_SHARED_COMPIZ_SOURCES
636 CompizUtils.cpp
637 PluginAdapter.cpp
638+ GnomeKeyGrabber.cpp
639 )
640
641 find_package (PkgConfig)
642+ pkg_check_modules (COMPIZ REQUIRED compiz)
643 pkg_check_modules (COMPIZ_OPENGL REQUIRED compiz-opengl)
644
645 add_library (unity-shared-compiz STATIC ${UNITY_SHARED_COMPIZ_SOURCES})
646
647 # This makes linker to include library dir in RUNPATH
648+ find_library (COMPIZ_LIB compiz_core ${COMPIZ_LIBDIR})
649 find_library (COMPIZ_OPENGL_LIB opengl ${COMPIZ_OPENGL_LIBDIR})
650- target_link_libraries (unity-shared-compiz ${LIBS} ${COMPIZ_OPENGL_LIB} ${COMPIZ_OPENGL_LDFLAGS})
651+ target_link_libraries (unity-shared-compiz
652+ ${LIBS}
653+ ${COMPIZ_LIB}
654+ ${COMPIZ_LDFLAGS}
655+ ${COMPIZ_OPENGL_LIB}
656+ ${COMPIZ_OPENGL_LDFLAGS})
657 add_dependencies (unity-shared-compiz unity-shared)
658
659 # bamf application manager
660
661=== added file 'unity-shared/GnomeKeyGrabber.cpp'
662--- unity-shared/GnomeKeyGrabber.cpp 1970-01-01 00:00:00 +0000
663+++ unity-shared/GnomeKeyGrabber.cpp 2014-02-11 20:09:36 +0000
664@@ -0,0 +1,268 @@
665+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
666+/*
667+ * Copyright (C) 2013 Canonical Ltd
668+ *
669+ * This program is free software: you can redistribute it and/or modify
670+ * it under the terms of the GNU General Public License version 3 as
671+ * published by the Free Software Foundation.
672+ *
673+ * This program is distributed in the hope that it will be useful,
674+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
675+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
676+ * GNU General Public License for more details.
677+ *
678+ * You should have received a copy of the GNU General Public License
679+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
680+ *
681+ * Authored by: William Hua <william.hua@canonical.com>
682+ */
683+
684+#include "GnomeKeyGrabberImpl.h"
685+
686+#include <NuxCore/Logger.h>
687+
688+namespace unity
689+{
690+DECLARE_LOGGER(logger, "unity.gnome");
691+
692+// Private implementation
693+namespace shell
694+{
695+std::string const DBUS_NAME = "org.gnome.Shell";
696+std::string const DBUS_INTERFACE = "org.gnome.Shell";
697+std::string const DBUS_OBJECT_PATH = "/org/gnome/Shell";
698+std::string const INTROSPECTION_XML =
699+R"(<node>
700+ <interface name='org.gnome.Shell'>
701+ <method name='GrabAccelerators'>
702+ <arg type='a(su)' direction='in' name='accelerators'/>
703+ <arg type='au' direction='out' name='actions'/>
704+ </method>
705+ <method name='GrabAccelerator'>
706+ <arg type='s' direction='in' name='accelerator'/>
707+ <arg type='u' direction='in' name='flags'/>
708+ <arg type='u' direction='out' name='action'/>
709+ </method>
710+ <method name='UngrabAccelerator'>
711+ <arg type='u' direction='in' name='action'/>
712+ <arg type='b' direction='out' name='success'/>
713+ </method>
714+ <signal name='AcceleratorActivated'>
715+ <arg type='u' name='action'/>
716+ <arg type='u' name='device'/>
717+ </signal>
718+ </interface>
719+</node>)";
720+}
721+
722+namespace testing
723+{
724+std::string const DBUS_NAME = "com.canonical.Unity.Test.GnomeKeyGrabber";
725+}
726+
727+GnomeKeyGrabber::Impl::Impl(CompScreen* screen, bool test_mode)
728+ : test_mode_(test_mode)
729+ , shell_server_(test_mode_ ? testing::DBUS_NAME : shell::DBUS_NAME)
730+ , screen_(screen)
731+ , current_action_id_(0)
732+{
733+ shell_server_.AddObjects(shell::INTROSPECTION_XML, shell::DBUS_OBJECT_PATH);
734+ shell_object_ = shell_server_.GetObject(shell::DBUS_INTERFACE);
735+ shell_object_->SetMethodsCallsHandler(sigc::mem_fun(this, &Impl::onShellMethodCall));
736+}
737+
738+GnomeKeyGrabber::Impl::~Impl()
739+{
740+ if (screen_)
741+ {
742+ for (auto& action : actions_)
743+ screen_->removeAction(&action);
744+ }
745+}
746+
747+unsigned int GnomeKeyGrabber::Impl::addAction(CompAction const& action, bool addressable)
748+{
749+ ++current_action_id_;
750+ actions_.push_back(action);
751+ action_ids_.push_back(current_action_id_);
752+
753+ if (addressable)
754+ {
755+ action_ids_by_action_[&action] = current_action_id_;
756+ actions_by_action_id_[current_action_id_] = &action;
757+ }
758+
759+ if (screen_)
760+ screen_->addAction(&actions_.back());
761+
762+ return current_action_id_;
763+}
764+
765+bool GnomeKeyGrabber::Impl::removeAction(CompAction const& action)
766+{
767+ auto i = action_ids_by_action_.find(&action);
768+ return i != action_ids_by_action_.end() && removeAction(i->second);
769+}
770+
771+bool GnomeKeyGrabber::Impl::removeAction(unsigned int action_id)
772+{
773+ auto i = std::find(action_ids_.begin(), action_ids_.end(), action_id);
774+
775+ if (i != action_ids_.end())
776+ {
777+ auto j = actions_.begin() + (i - action_ids_.begin());
778+ auto k = actions_by_action_id_.find(action_id);
779+
780+ if (screen_)
781+ screen_->removeAction(&(*j));
782+
783+ if (k != actions_by_action_id_.end())
784+ {
785+ action_ids_by_action_.erase(k->second);
786+ actions_by_action_id_.erase(k);
787+ }
788+
789+ action_ids_.erase(i);
790+ actions_.erase(j);
791+ return true;
792+ }
793+
794+ return false;
795+}
796+
797+GVariant* GnomeKeyGrabber::Impl::onShellMethodCall(std::string const& method, GVariant* parameters)
798+{
799+ LOG_DEBUG(logger) << "Called method '" << method << "'";
800+
801+ if (method == "GrabAccelerators")
802+ {
803+ if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(a(su))")))
804+ {
805+ GVariant* variant;
806+ GVariantBuilder builder;
807+ GVariantIter* iterator;
808+ gchar const* accelerator;
809+ guint flags;
810+
811+ g_variant_builder_init(&builder, G_VARIANT_TYPE("au"));
812+ g_variant_get(parameters, "(a(su))", &iterator);
813+
814+ while (g_variant_iter_next(iterator, "(&su)", &accelerator, &flags))
815+ g_variant_builder_add(&builder, "u", grabAccelerator(accelerator, flags));
816+
817+ g_variant_iter_free(iterator);
818+ variant = g_variant_builder_end(&builder);
819+ return g_variant_new_tuple(&variant, 1);
820+ }
821+ else
822+ LOG_WARN(logger) << "Expected arguments of type (a(su))";
823+ }
824+ else if (method == "GrabAccelerator")
825+ {
826+ if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(su)")))
827+ {
828+ GVariant* variant;
829+ gchar const* accelerator;
830+ guint flags;
831+
832+ g_variant_get(parameters, "(&su)", &accelerator, &flags);
833+ variant = g_variant_new_uint32(grabAccelerator(accelerator, flags));
834+ return g_variant_new_tuple(&variant, 1);
835+ }
836+ else
837+ LOG_WARN(logger) << "Expected arguments of type (su)";
838+ }
839+ else if (method == "UngrabAccelerator")
840+ {
841+ if (g_variant_is_of_type(parameters, G_VARIANT_TYPE("(u)")))
842+ {
843+ GVariant* variant;
844+ guint action;
845+
846+ g_variant_get(parameters, "(u)", &action);
847+ variant = g_variant_new_boolean(removeAction(action));
848+ return g_variant_new_tuple(&variant, 1);
849+ }
850+ else
851+ LOG_WARN(logger) << "Expected arguments of type (u)";
852+ }
853+
854+ return nullptr;
855+}
856+
857+unsigned int GnomeKeyGrabber::Impl::grabAccelerator(char const* accelerator, unsigned int flags)
858+{
859+ CompAction action;
860+ action.keyFromString(accelerator);
861+
862+ if (!isActionPostponed(action))
863+ {
864+ action.setState(CompAction::StateInitKey);
865+ action.setInitiate([this](CompAction* action, CompAction::State state, CompOption::Vector& options) {
866+ activateAction(action, 0);
867+ return true;
868+ });
869+ }
870+ else
871+ {
872+ action.setState(CompAction::StateInitKey | CompAction::StateTermKey);
873+ action.setTerminate([this](CompAction* action, CompAction::State state, CompOption::Vector& options) {
874+ if (state & CompAction::StateTermTapped)
875+ {
876+ activateAction(action, 0);
877+ return true;
878+ }
879+
880+ return false;
881+ });
882+ }
883+
884+ return addAction(action, false);
885+}
886+
887+void GnomeKeyGrabber::Impl::activateAction(CompAction const* action, unsigned int device) const
888+{
889+ ptrdiff_t i = action - &actions_.front();
890+
891+ if (0 <= i && i < static_cast<ptrdiff_t>(action_ids_.size()))
892+ shell_object_->EmitSignal("AcceleratorActivated", g_variant_new("(uu)", action_ids_[i], device));
893+}
894+
895+bool GnomeKeyGrabber::Impl::isActionPostponed(CompAction const& action) const
896+{
897+ int keycode = action.key().keycode();
898+ return keycode == 0 || modHandler->keycodeToModifiers(keycode) != 0;
899+}
900+
901+// Public implementation
902+
903+GnomeKeyGrabber::GnomeKeyGrabber(CompScreen* screen)
904+ : impl_(new Impl(screen))
905+{
906+}
907+
908+GnomeKeyGrabber::GnomeKeyGrabber(CompScreen* screen, TestMode const& dummy)
909+ : impl_(new Impl(screen, true))
910+{
911+}
912+
913+GnomeKeyGrabber::~GnomeKeyGrabber()
914+{
915+}
916+
917+CompAction::Vector& GnomeKeyGrabber::getActions()
918+{
919+ return impl_->actions_;
920+}
921+
922+void GnomeKeyGrabber::addAction(CompAction const& action)
923+{
924+ impl_->addAction(action);
925+}
926+
927+void GnomeKeyGrabber::removeAction(CompAction const& action)
928+{
929+ impl_->removeAction(action);
930+}
931+
932+} // namespace unity
933
934=== added file 'unity-shared/GnomeKeyGrabber.h'
935--- unity-shared/GnomeKeyGrabber.h 1970-01-01 00:00:00 +0000
936+++ unity-shared/GnomeKeyGrabber.h 2014-02-11 20:09:36 +0000
937@@ -0,0 +1,54 @@
938+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
939+/*
940+* Copyright (C) 2013 Canonical Ltd
941+*
942+* This program is free software: you can redistribute it and/or modify
943+* it under the terms of the GNU General Public License version 3 as
944+* published by the Free Software Foundation.
945+*
946+* This program is distributed in the hope that it will be useful,
947+* but WITHOUT ANY WARRANTY; without even the implied warranty of
948+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
949+* GNU General Public License for more details.
950+*
951+* You should have received a copy of the GNU General Public License
952+* along with this program. If not, see <http://www.gnu.org/licenses/>.
953+*
954+* Authored by: William Hua <william.hua@canonical.com>
955+*/
956+
957+#ifndef __GNOME_KEY_GRABBER_H__
958+#define __GNOME_KEY_GRABBER_H__
959+
960+#include <core/core.h>
961+
962+namespace unity
963+{
964+
965+class GnomeKeyGrabber
966+{
967+public:
968+
969+ typedef std::shared_ptr<GnomeKeyGrabber> Ptr;
970+
971+ explicit GnomeKeyGrabber(CompScreen* screen);
972+ virtual ~GnomeKeyGrabber();
973+
974+ CompAction::Vector& getActions();
975+ void addAction(CompAction const& action);
976+ void removeAction(CompAction const& action);
977+
978+protected:
979+
980+ struct TestMode {};
981+ GnomeKeyGrabber(CompScreen* screen, TestMode const& dummy);
982+
983+private:
984+
985+ struct Impl;
986+ std::unique_ptr<Impl> impl_;
987+};
988+
989+} // namespace unity
990+
991+#endif // __GNOME_KEY_GRABBER_H__
992
993=== added file 'unity-shared/GnomeKeyGrabberImpl.h'
994--- unity-shared/GnomeKeyGrabberImpl.h 1970-01-01 00:00:00 +0000
995+++ unity-shared/GnomeKeyGrabberImpl.h 2014-02-11 20:09:36 +0000
996@@ -0,0 +1,64 @@
997+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
998+/*
999+* Copyright (C) 2013 Canonical Ltd
1000+*
1001+* This program is free software: you can redistribute it and/or modify
1002+* it under the terms of the GNU General Public License version 3 as
1003+* published by the Free Software Foundation.
1004+*
1005+* This program is distributed in the hope that it will be useful,
1006+* but WITHOUT ANY WARRANTY; without even the implied warranty of
1007+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1008+* GNU General Public License for more details.
1009+*
1010+* You should have received a copy of the GNU General Public License
1011+* along with this program. If not, see <http://www.gnu.org/licenses/>.
1012+*
1013+* Authored by: William Hua <william.hua@canonical.com>
1014+*/
1015+
1016+#ifndef __GNOME_KEY_GRABBER_IMPL_H__
1017+#define __GNOME_KEY_GRABBER_IMPL_H__
1018+
1019+#include "GnomeKeyGrabber.h"
1020+
1021+#include <UnityCore/GLibDBusProxy.h>
1022+#include <UnityCore/GLibDBusServer.h>
1023+
1024+#include <unordered_map>
1025+
1026+namespace unity
1027+{
1028+
1029+struct GnomeKeyGrabber::Impl
1030+{
1031+ bool test_mode_;
1032+
1033+ glib::DBusServer shell_server_;
1034+ glib::DBusObject::Ptr shell_object_;
1035+
1036+ CompScreen* screen_;
1037+ CompAction::Vector actions_;
1038+ std::vector<unsigned int> action_ids_;
1039+ unsigned int current_action_id_;
1040+
1041+ std::unordered_map<CompAction const*, unsigned int> action_ids_by_action_;
1042+ std::unordered_map<unsigned int, CompAction const*> actions_by_action_id_;
1043+
1044+ explicit Impl(CompScreen* screen, bool test_mode = false);
1045+ ~Impl();
1046+
1047+ unsigned int addAction(CompAction const& action, bool addressable = true);
1048+ bool removeAction(CompAction const& action);
1049+ bool removeAction(unsigned int action_id);
1050+
1051+ GVariant* onShellMethodCall(std::string const& method, GVariant* parameters);
1052+ unsigned int grabAccelerator(char const* accelerator, unsigned int flags);
1053+ void activateAction(CompAction const* action, unsigned int device) const;
1054+
1055+ bool isActionPostponed(CompAction const& action) const;
1056+};
1057+
1058+} // namespace unity
1059+
1060+#endif // __GNOME_KEY_GRABBER_IMPL_H__
1061
1062=== modified file 'unity-standalone/CMakeLists.txt'
1063--- unity-standalone/CMakeLists.txt 2012-12-19 14:39:56 +0000
1064+++ unity-standalone/CMakeLists.txt 2014-02-11 20:09:36 +0000
1065@@ -27,4 +27,5 @@
1066 panel-lib
1067 unity-shared
1068 unity-shared-bamf
1069+ unity-shared-compiz
1070 unity-shared-standalone)