appmenu-qt

Merge lp:~gnumdk/appmenu-qt/fix-1906008 into lp:appmenu-qt

  1. fix-1906008
  2. Merge into trunk
Proposed by Cédric Bellegarde
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 51
Merged at revision: 50
Proposed branch: lp:~gnumdk/appmenu-qt/fix-1906008
Merge into: lp:appmenu-qt
Diff against target: 120 lines (+68/-5)
3 files modified
src/CMakeLists.txt (+4/-0)
src/appmenuplatformmenubar.cpp (+8/-5)
src/com.canonical.AppMenu.Registrar.xml (+56/-0)
To merge this branch: bzr merge lp:~gnumdk/appmenu-qt/fix-1906008
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+142914@code.launchpad.net

Description of the change

Here a patch fixing issues in KDE 4.10...

It make appmenu-qt use async dbus introspection preventing dbus locks...

http://lists.kde.org/?l=kde-core-devel&m=135729000730025&w=4

Should be cool to be applied and released for KDE 4.10 ;)

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote :

Doesn't build

tsdgeos_work@xps:~/fix-1906008$ make
make[2]: *** No rule to make target `src/com.canonical.AppMenu.Registrar.xml', needed by `src/registrar.cpp'. Stop.
make[1]: *** [src/CMakeFiles/appmenu-qt.dir/all] Error 2
make: *** [all] Error 2

review: Needs Fixing
lp:~gnumdk/appmenu-qt/fix-1906008 updated
51. By Cédric Bellegarde

Add missing file

Revision history for this message
Cédric Bellegarde (gnumdk) wrote :

Should be fixed by revision 51.

Revision history for this message
Albert Astals Cid (aacid) wrote :

I'm not sure the static is a good idea, what if the registrar changes (it can happen as there is the m_registrarWatcher), will the static send the messages to the new one?

Revision history for this message
Cédric Bellegarde (gnumdk) wrote :

Yes, it's working, the static just send dbus message to "REGISTRAR_SERVICE, REGISTRAR_PATH"

$ qdbus org.kde.kded /kded org.kde.kded.unloadModule appmenu

registrar is unloaded

$ qdbus org.kde.kded /kded org.kde.kded.loadModule appmenu

registrar is loaded and get menus from already running applications

Thanks for pointing me to m_registrarWatcher, there is another bug to fix :)

Revision history for this message
Albert Astals Cid (aacid) wrote :

Looks good then

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/CMakeLists.txt'
2--- src/CMakeLists.txt 2012-02-27 14:27:17 +0000
3+++ src/CMakeLists.txt 2013-01-14 09:49:25 +0000
4@@ -15,6 +15,10 @@
5 ${QT_QTXML_INCLUDE_DIR}
6 )
7
8+qt4_add_dbus_interface(appmenu_qt_SRCS com.canonical.AppMenu.Registrar.xml
9+ registrar)
10+
11+
12 qt4_automoc(${appmenu_qt_SRCS})
13
14 link_directories(
15
16=== modified file 'src/appmenuplatformmenubar.cpp'
17--- src/appmenuplatformmenubar.cpp 2012-04-04 16:06:06 +0000
18+++ src/appmenuplatformmenubar.cpp 2013-01-14 09:49:25 +0000
19@@ -15,6 +15,7 @@
20 along with appmenu-qt. If not, see <http://www.gnu.org/licenses/>.
21 */
22 #include "appmenuplatformmenubar.h"
23+#include "registrar.h"
24
25 // dbusmenu-qt
26 #include <dbusmenuexporter.h>
27@@ -393,6 +394,8 @@
28
29 bool MenuBarAdapter::registerWindow()
30 {
31+ static com::canonical::AppMenu::Registrar *registrar = 0;
32+
33 if (!m_menuBar->window()) {
34 WARN << "No parent for this menubar";
35 return false;
36@@ -403,9 +406,8 @@
37 return true;
38 }
39
40- QDBusInterface host(REGISTRAR_SERVICE, REGISTRAR_PATH, REGISTRAR_IFACE);
41- if (!host.isValid()) {
42- return false;
43+ if (!registrar) {
44+ registrar = new com::canonical::AppMenu::Registrar(REGISTRAR_SERVICE, REGISTRAR_PATH, QDBusConnection::sessionBus(), 0);
45 }
46
47 Q_FOREACH(QAction *action, m_menuBar->actions()) {
48@@ -428,8 +430,9 @@
49 }
50
51 m_registeredWinId = winId;
52- QVariant path = QVariant::fromValue<QDBusObjectPath>(QDBusObjectPath(m_objectPath));
53- host.asyncCall(QLatin1String("RegisterWindow"), QVariant(winId), path);
54+ if (registrar) {
55+ registrar->RegisterWindow(winId, QDBusObjectPath(m_objectPath));
56+ }
57 return true;
58 }
59
60
61=== added file 'src/com.canonical.AppMenu.Registrar.xml'
62--- src/com.canonical.AppMenu.Registrar.xml 1970-01-01 00:00:00 +0000
63+++ src/com.canonical.AppMenu.Registrar.xml 2013-01-14 09:49:25 +0000
64@@ -0,0 +1,56 @@
65+<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
66+<node xmlns:dox="http://www.ayatana.org/dbus/dox.dtd">
67+ <dox:d><![CDATA[
68+ @mainpage
69+  
70+ An interface to register menus that are associated with a window in an application.  The
71+ main interface is docuemented here: @ref com::canonical::AppMenu::Registrar.
72+     
73+ The actual menus are transported using the dbusmenu protocol which is available
74+ here: @ref com::canonical::dbusmenu.
75+ ]]></dox:d>
76+ <interface name="com.canonical.AppMenu.Registrar" xmlns:dox="http://www.ayatana.org/dbus/dox.dtd">
77+ <dox:d>
78+ An interface to register a menu from an application's window to be displayed in another
79+ window.  This manages that association between XWindow Window IDs and the dbus
80+ address and object that provides the menu using the dbusmenu dbus interface.
81+ </dox:d>
82+ <method name="RegisterWindow">
83+ <dox:d><![CDATA[
84+ Associates a dbusmenu with a window
85+      
86+ /note this method assumes that the connection from the caller is the DBus connection
87+ to use for the object.  Applications that use multiple DBus connections will need to
88+ ensure this method is called with the same connection that implmenets the object.
89+ ]]></dox:d>
90+ <arg name="windowId" type="u" direction="in">
91+ <dox:d>The XWindow ID of the window</dox:d>
92+ </arg>
93+ <arg name="menuObjectPath" type="o" direction="in">
94+ <dox:d>The object on the dbus interface implementing the dbusmenu interface</dox:d>
95+ </arg>
96+ </method>
97+ <method name="UnregisterWindow">
98+ <dox:d>
99+ A method to allow removing a window from the database. Windows will also be removed
100+ when the client drops off DBus so this is not required. It is polite though. And
101+ important for testing.
102+ </dox:d>
103+ <arg name="windowId" type="u" direction="in">
104+ <dox:d>The XWindow ID of the window</dox:d>
105+ </arg>
106+ </method>
107+ <method name="GetMenuForWindow">
108+ <dox:d>Gets the registered menu for a given window ID.</dox:d>
109+ <arg name="windowId" type="u" direction="in">
110+ <dox:d>The XWindow ID of the window to get</dox:d>
111+ </arg>
112+ <arg name="service" type="s" direction="out">
113+ <dox:d>The address of the connection on DBus (e.g. :1.23 or org.example.service)</dox:d>
114+ </arg>
115+ <arg name="menuObjectPath" type="o" direction="out">
116+ <dox:d>The path to the object which implements the com.canonical.dbusmenu interface.</dox:d>
117+ </arg>
118+ </method>
119+ </interface>
120+</node>

Subscribers

People subscribed via source and target branches

to all changes: