Summary: | [Spec 1.2] Support image-path | ||
---|---|---|---|
Product: | [Plasma] plasmashell | Reporter: | Konstantin <ria.freelander> |
Component: | Notifications | Assignee: | Martin Klapetek <mklapetek> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | kde |
Priority: | NOR | ||
Version: | 5.2.1 | ||
Target Milestone: | 1.0 | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/plasma-workspace/8d4dd0bb9b491c2025664b86ded4a76d41d56d13 | Version Fixed In: | |
Sentry Crash Report: | |||
Attachments: |
attachment-19219-0.html
Patch to support dashed hints also Updated version. |
Description
Konstantin
2015-03-05 22:34:03 UTC
Thanks for the report Can you post a link to the spec? Created attachment 91443 [details] attachment-19219-0.html Here it is: https://people.gnome.org/~mccann/docs/notification-spec/notification-spec-latest.html And here the same: https://developer.gnome.org/notification-spec/ On Mar 6, 2015 5:02 AM, "Martin Klapetek" <mklapetek@kde.org> wrote: > https://bugs.kde.org/show_bug.cgi?id=344885 > > --- Comment #1 from Martin Klapetek <mklapetek@kde.org> --- > Thanks for the report > > Can you post a link to the spec? > > -- > You are receiving this mail because: > You reported the bug. > Right, so that's Gnome's specific extension of the notification standard and we've already decided that we won't support it as it's fully in Gnome's control and can change any time without giving notice, can break any time Gnome decides to change things etc. If this should become more widespread, it should go to the proper xdg places. Sorry. Some note about notification spec. It is not a gnome extension. Here is a proof in XDG mailing list: http://lists.freedesktop.org/archives/xdg/2015-February/013456.html And here is spec in libnotify git tree: https://github.com/GNOME/libnotify/blob/master/docs/notification-spec.xml It's an extension that was designed by Gnome and Gnome only (afaik, for sure KDE was not involved there). As such, it still is Gnome's extension. It will not widespread if all developers will count it as a GNOME extension. Can I see KDE version of org.freedesktop.Notifications spec? I want to make my app compatible with KDE also. It will not widespread if it will be only under Gnome's control. I don't mean to be mean but they do tend to remove things randomly here and there. And that's perfectly fine, but not if things are supposed to be supported by others (namely we've hit a roadblock with their theming support as well as the CSD and all the bugs it caused in Plasma while Gnome's stance being "not our problem"). I sure don't want to end up at the same place wrt notifications. It should go through proper collaboration and review and things. The original spec is at http://www.galago-project.org/specs/notification/ So, you use version 0.9 of this spec. Ok. But this is not request for removal, but request for adding new hint. This is logical, AFAIK, because in 0.9 it is image_data (image-data in 1.2), and image-path is simply a way to set image-data to fdo compilant icon name (or filesystem path). I can add support of this myself, I think this will be easy. I'd like to avoid implementing custom things that are not in the spec. In fact, icons should always be installed into proper places if you want to ensure things will all work correctly. It is in the spec. Your daemon reports that it supports version 1.1 of the spec: ``` // FIXME: Signature is ugly QString NotificationsEngine::GetServerInformation(QString& vendor, QString& version, QString& specVersion) { vendor = "KDE"; version = "2.0"; // FIXME specVersion = "1.1"; return "Plasma"; } ``` Just checked version 1.1 of the spec in libnotify git - image_path was there. Change between 1.1 and 1.2 is cosmetic: support dashes-named properties instead of underscores-named. And to note: image-path property is not only about paths. It also about freedesktop.org compilant icon names, as spec (in libnotify git) stats. For example, one can set image-path to "audio-volume-mute", and daemon must shows icon of strikethrough speaker on notification. and so, in plasma it has: ``` if (hints.contains("image_data")) { QDBusArgument arg = hints["image_data"].value<QDBusArgument>(); image = decodeNotificationSpecImageHint(arg); } else if (hints.contains("image_path")) { QString path = findImageForSpecImagePath(hints["image_path"].toString()); if (!path.isEmpty()) { image.load(path); } } else if (hints.contains("icon_data")) { // This hint was in use in version 1.0 of the spec but has been // replaced by "image_data" in version 1.1. We need to support it for // users of the 1.0 version of the spec. QDBusArgument arg = hints["icon_data"].value<QDBusArgument>(); image = decodeNotificationSpecImageHint(arg); } ``` Created attachment 91448 [details]
Patch to support dashed hints also
Here is plasma-workspace patch to adopt a dahsed hints.
plasma already has support image_path property. All work that was needed - adopt it to change underscore into a dash. Do you like it?
Hah, so we already even have support for that, who knew. Why don't you just use image_path then? Wait, do I understand it correctly that 1.2 /removed/ the underscore values? I use GLib API (in fact, in XFCE), and GLib sends notifications according by version 1.2. it sends an icon in image-path property. When I discover it, I already has a discussion in a GLib bugtracker about it. Here is a link: https://bugzilla.gnome.org/show_bug.cgi?id=745634. Just for summary: GLib does not want to use "deprecated" undersore values and app icon. It is not removed from spec now, but cleanly stated that newly written programs (from 28 October 2010) must support a dashed values. Underscores only stays here for compatibility reason, like icon_data. Created attachment 91449 [details]
Updated version.
It is an updated version with correct spec_version
Sigh, that's exactly what the server version is for. If it detects older version (which it clearly does, unless it skips it altogether), it should not use features newer than the version it actually talks to. Imho that's GNotification bug. I don't think it's that simple to just raise the version and be done; I'll look over what the 1.1/1.2 all expects to have and see what all is missing and what should we do with it. GNotification skips version altogether, assumes than 1.2 is supported. Additions of 1.2: 1. Dashes replaces uderscores everywhere 2. "resident" boolean hint. It cleanly stated that notification will not automatically dismissed when action is activated. 3. "persistence" capability and "transient" hint. It does not makes sense to KDE, because KDE notification daemon does not return "persistence" capability. But if you want, you can implement it. Well persistence should be done by setting the timeout to 0, as the spec says. It makes me really reluctant however to fix our server to follow Gnome's extension only because GNotification does not work properly. That's really not our bug. But why dashed names is GNOME extension? I does not understand. I think this is freedesktop spec, not GNOME. XDG mailing list says that libnotify is reference implementation of org.freedesktop.Notifications spec and spec is hosted in libnotify git. And next, GNOME does not use org.freedesktop.Notifications anymore. It have private bus named org.gtk.Notifications. 1.2 persistence means that notifications must stored across daemon restarts. For example, i send "resident" notification to daemon, kill it and start it, notification must shown again (or be in daemon database and accessible by user). "transient" hint means that notification follows old behaviour (removed after daemon kill) Proper xdg standards additions get proper xdg reviews (or at least I'd like to think so). The fact that it's hosted in Gnome's git and actually no KDE developer has (write) access to it and changes have to go through Gnome's review (that's my assumption though), makes it a Gnome extension. If it's claimed as an xdg extension, freedesktop.org is still there with all the other specs, I see no reason why the notification one could not join. Citation from Sanel Zukan in XDG mailing list: ``` Because fd.o admin isn't responsive enough when there are problems. Which happens often with the specs. And because it was always hosted within libnotify. ``` And next, galago's last spec version was 0.9. KDE supports 1.1, dashed values added in 1.2. Will dashed names be supported in KDE 5.3? As I said already, first I need to look what is all missing. I will not declare support for something when we don't actually support it. Second, it's GNotification's bug. Added GLib bugreport for GNotifications: https://bugzilla.gnome.org/show_bug.cgi?id=745749 Martin, shall we merge the patch; even if we just handle image-path without bumping the spec. The patch itself is fine. Ok. Git commit 8d4dd0bb9b491c2025664b86ded4a76d41d56d13 by David Edmundson. Committed on 08/04/2015 at 10:49. Pushed by davidedmundson into branch 'master'. Support hints from notification spec 1.2 Even though the specification makes it perfectly clear that you should query the notification server and only use supported hints, gnome and libnotify does not. Given the specification has changed in a non-compatiable way, this causes image-data/image-path to break. This patch supports 1.2 fields Submitted by: Konstantin Reviewed-by: David Edmundson M +12 -1 dataengines/notifications/notificationsengine.cpp http://commits.kde.org/plasma-workspace/8d4dd0bb9b491c2025664b86ded4a76d41d56d13 The gnome devs seemed to have "forgotten" about making it a specification on fd.o I've opened a new bug at: https://bugzilla.gnome.org/show_bug.cgi?id=748145 |