search

sugarlabs / sugar

Sugar GTK shell
GNU General Public License v3.0
252 stars 240 forks source link

notifications: Fix signature of GetServerInformation #944

Closed The-Compiler closed 2 years ago

The-Compiler commented 3 years ago

See https://specifications.freedesktop.org/notification-spec/latest/index.html, specifically the GetServerInformation signature.

I didn't test this myself as I only found this by accident while reviewing different notification daemons. You can test it using dbus-send --session --print-reply --dest=org.freedesktop.Notifications /org/freedesktop/Notifications org.freedesktop.Notifications.GetServerInformation which will show a TypeError without this change.

chimosky commented 3 years ago

Don't really know how to test this so I watched the dbus session with dbus-monitor and running the above gives this output

method call time=1618363367.690996 sender=:1.30 -> destination=org.freedesktop.DBus serial=1 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=Hello
method return time=1618363367.691044 sender=org.freedesktop.DBus -> destination=:1.30 serial=1 reply_serial=1
   string ":1.30"
signal time=1618363367.691067 sender=org.freedesktop.DBus -> destination=(null destination) serial=60 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameOwnerChanged
   string ":1.30"
   string ""
   string ":1.30"
signal time=1618363367.691100 sender=org.freedesktop.DBus -> destination=:1.30 serial=2 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameAcquired
   string ":1.30"
signal time=1618363367.693877 sender=org.freedesktop.DBus -> destination=:1.30 serial=5 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameLost
   string ":1.30"
signal time=1618363367.693924 sender=org.freedesktop.DBus -> destination=(null destination) serial=61 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameOwnerChanged
   string ":1.30"
   string ":1.30"
   string ""

@quozl how should this be tested?

The-Compiler commented 3 years ago

@chimosky See my PR description above for a suggestion on how to test this :slightly_smiling_face:

chimosky commented 3 years ago

@chimosky See my PR description above for a suggestion on how to test this slightly_smiling_face

Oh I did test with that, my reply above was using dbus-monitor while testing with that, should've posted the output of the command when I ran it.

dbus[1004]: arguments to dbus_message_new_method_call() were incorrect, assertion "_dbus_check_is_valid_path (path)" failed in file ../../../dbus/dbus-message.c line 1366.
This is normally a bug in some application using the D-Bus library.

  D-Bus not built with -rdynamic so unable to print a backtrace
Aborted
The-Compiler commented 3 years ago

I've never seen that error message, no idea what it means - but I assume you see that without my changes too?

chimosky commented 3 years ago

I've never seen that error message, no idea what it means - but I assume you see that without my changes too?

Yeah I do.