Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StatusNotifierItem: announce version and status #2599

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rilian-la-te
Copy link
Contributor

Fixes #2598

According to https://bazaar.launchpad.net/~dbusmenu-team/libdbusmenu/trunk.16.10/view/head:/libdbusmenu-glib/dbus-menu.xml, there should be 4 properties, but I am implemented 2, which makes sense.

And here https://bazaar.launchpad.net/~dbusmenu-team/libdbusmenu/trunk.16.10/view/head:/libdbusmenu-glib/server.c
you can see, than libdbusmenu announces version 3.

@cschramm
Copy link
Member

cschramm commented Jan 4, 2025

Please provide some background on where you're coming from. I'm not keen to adopt a random version number from that library.

As you know, there is no actual specification, just that reference implementation if you'd like to call it that. Digging into its history makes me expect version 3 to imply support for event groups. We do not have that.

Looking at https://bazaar.launchpad.net/~dbusmenu-team/libdbusmenu/trunk.16.10/view/head:/libdbusmenu-glib/client.c#L1112, you see that all those properties are considered optional and that the version – if available – is actually used to check for >= 3 to enable event group support and nothing else.

I would not expect clients to require a version property and they definitely do not if they use libdbusmenu-glib. I'd also expect issues if we advertise version 3 to that library and thus most clients.

@rilian-la-te
Copy link
Contributor Author

rilian-la-te commented Jan 5, 2025

@cschramm you are right about version 3, and there is no support of event groups.

I need to know how to check out whether interface or functional or not, because sometimes tray clients did not announce menu at all, and it need to be checked, and check version field is best what I can think, because any of known implementations announce non-zero version, except Blueman one.

Copy link

sonarqubecloud bot commented Jan 5, 2025

@cschramm
Copy link
Member

cschramm commented Jan 7, 2025

I'm not sure I can follow. You seem to be coming from vala-panel-appmenu and as I see you already removed the version check there: https://gitlab.com/vala-panel-project/vala-panel-appmenu/-/commit/218bb4aaf1a1d9fcd89a71a14c8bf99cfd94ba1e.

If I get you right, the presence of a menu can be speculative at that point and you need to check if it holds as $something has such a menu but does not advertise it as org.freedesktop.StatusNotifierItem.Menu?

Two questions come to my mind:

  • Your importer object knows an object path. Where does that come from? Is it simply that of the StatusNotifierItem?
  • If $something does not advertise the menu, does it get picked up by other StatusNotifierHosts?

In any case, the goto solution would be introspection, assuming that $something provides it. Otherwise, you can still just try to use the interface, e.g. calling AboutToShow. libdbusmenu also just expects the interface and certain methods to be available on the provided objects and otherwise its calls will fail. Of course, it's up to the library's caller to specify the objects and it could possibly pre-check them (if they are speculative), but that's not encouraged in any way.

It also makes sense to only apply such extra checks in the speculative case and not in cases like blueman where the menu got advertised.

I don't know if there are other implementations out there that do not provide a version, but we have not had any issues with that so far – in line with libdbusmenu's behavior –, so requiring one seems risky. Also, the value appears pointless; we're neither 1 (where the property would even be version, not Version, by the way) nor 3 and there are so many releases of libdbusmenu that advertise version 2 but provide different interfaces (e.g. both version and Version variants), that I would not call that an information. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StatusNotifierItem: announce version 3 of the DBusMenu protocol
2 participants