D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added a comment. In D27464#625403 , @broulik wrote: > Didn't we have a dedicated protocol for window menu in plasma-integration? Or is this just for reading? We surely don't want all apps to use plasma surface interface for announcing the

D27464: Add application menu dbus paths to org_kde_plasma_window interface

broulik added a comment. In D27464#625405 , @zzag wrote: > Could you please clarify what you mean by "plasma surface interface"? The thing we use in plasmashell to position panels and everything absolutely REPOSITORY R127 KWayland

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added a comment. In D27464#625403 , @broulik wrote: > Didn't we have a dedicated protocol for window menu in plasma-integration? That protocol is for announcing global menus. > Or is this just for reading? Yes, it's for

D27464: Add application menu dbus paths to org_kde_plasma_window interface

broulik added a comment. Didn't we have a dedicated protocol for window menu in plasma-integration? Or is this just for reading? We surely don't want all apps to use plasma surface interface for announcing the global menu?

D27464: Add application menu dbus paths to org_kde_plasma_window interface

This revision was automatically updated to reflect the committed changes. Closed by commit R127:dddf9d1ac3f2: Add application menu dbus paths to org_kde_plasma_window interface (authored by cblack). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack added a dependent revision: D27818: [wayland/AbstractClient] Broadcast application menu events. REPOSITORY R127 KWayland BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag, davidedmundson Cc: davidedmundson,

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag accepted this revision. zzag added a comment. This revision is now accepted and ready to land. Thanks. Even though the patch is accepted, please do not land it yet. https://phabricator.kde.org/D27464#621521 REPOSITORY R127 KWayland BRANCH cblack/appmenu-listener REVISION DETAIL

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76886. cblack added a comment. Add missing documentation REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=76885=76886 BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added a comment. Looks good to me, but please don't land without corresponding KWin patch. INLINE COMMENTS > plasmawindowmanagement.h:700 > +/** > + * TODO: Documentation. > + **/ You missed this one. REPOSITORY R127 KWayland REVISION DETAIL

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76885. cblack added a comment. Address feedback REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=76811=76885 BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464 AFFECTED

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added inline comments. INLINE COMMENTS > plasmawindowmanagement.cpp:372 > + > +Q_UNUSED(window); > +} Please move it to the top of the method and remove the semicolon. > plasmawindowmanagement.h:526-527 > > +QString applicationMenuServiceName() const; > +QString

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack added a comment. @zzag does anything else look off to you? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag, davidedmundson Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

davidedmundson accepted this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag, davidedmundson Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76811. cblack added a comment. Coding style REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=76805=76811 BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464 AFFECTED FILES

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack marked an inline comment as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added inline comments. INLINE COMMENTS > plasmawindowmanagement_interface.cpp:85 > void setGeometry(const QRect ); > +void setApplicationMenuPaths(const QString& service, const QString& > object); > wl_resource *resourceForParent(PlasmaWindowInterface *parent, > wl_resource

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack marked 3 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76805. cblack added a comment. Tick up PWM version REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=76804=76805 BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464 AFFECTED

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack marked 6 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76804. cblack added a comment. Address feedback REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=76035=76804 BRANCH cblack/appmenu-listener REVISION DETAIL https://phabricator.kde.org/D27464 AFFECTED

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag requested changes to this revision. zzag added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > plasmawindowmanagement.cpp:363 > > +void PlasmaWindow::Private::appmenuChangedCallback(void *data, > org_kde_plasma_window *window, const char* service_name,

D27464: Add application menu dbus paths to org_kde_plasma_window interface

zzag added inline comments. INLINE COMMENTS > plasma-window-management.xml:20 > > > version="10" > plasma-window-management.xml:87 > > - > + > version="10" > plasma-window-management.xml:312 > > + > + and this one should be > davidedmundson

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack added a comment. Would the version number already being 9 in old versions of KWayland cause any issues? I have a feeling old KWayland versions may not like it when they're sent events they don't recognize, unless libwayland/KWayland handle that usecase. REPOSITORY R127 KWayland

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack added inline comments. INLINE COMMENTS > davidedmundson wrote in plasmawindowmanagement_interface.cpp:129 > how is this 9 already? Git blame shows that it's been 9 since 2018. Looks like someone made a mistake a while back? REPOSITORY R127 KWayland REVISION DETAIL

D27464: Add application menu dbus paths to org_kde_plasma_window interface

davidedmundson added a comment. This is a nice simple example of something with a version bump: https://phabricator.kde.org/D27535 for where to change registry. I was wrong about display, there is a number you need to change, but apparently it's on v9 already, so you don't need to do

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack added a comment. In D27464#614479 , @davidedmundson wrote: > Good start. The direction makes sense. > > display.cpp and registry.cpp need updating to the new version number. Otherwise we won't be able to use this new method. > >

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack marked an inline comment as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 76035. cblack added a comment. Use utf8 encoding on client side REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=75855=76035 BRANCH cblack/appmenu-listener REVISION DETAIL

D27464: Add application menu dbus paths to org_kde_plasma_window interface

davidedmundson added a comment. Good start. The direction makes sense. display.cpp and registry.cpp need updating to the new version number. Otherwise we won't be able to use this new method. It'd be great if you could update the unit test too. INLINE COMMENTS >

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack updated this revision to Diff 75855. cblack added a comment. Figure out how to use arc correctly REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27464?vs=75854=75855 BRANCH cblack/appmenu-listener REVISION DETAIL

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack planned changes to this revision. cblack added a comment. whoops, I seem to have forgotten how to use arc REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D27464 To: cblack, #kwin, zzag Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,

D27464: Add application menu dbus paths to org_kde_plasma_window interface

cblack created this revision. cblack added reviewers: KWin, zzag. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. cblack requested review of this revision. REVISION SUMMARY This patch adds an interface allowing a compositor to send the service name and