Merge pull request #3040 from Elv13/notif_icons_v3

[RFC] Redesign how notification icons are handled.

lib/gears/object.lua

@@ -134,7 +134,8 @@ function object:emit_signal(name, ...)
     end
 end
 
-function object._setup_class_signals(t)
+function object._setup_class_signals(t, args)
+    args = args or {}
     local conns = {}
 
     function t.connect_signal(name, func)
@@ -143,6 +144,18 @@ function object._setup_class_signals(t)
         table.insert(conns[name], func)
     end
 
+    -- A variant of emit_signal which stops once a condition is met.
+    if args.allow_chain_of_responsibility then
+        function t._emit_signal_if(name, condition, ...)
+            assert(name)
+            for _, func in pairs(conns[name] or {}) do
+                if condition(...) then return end
+                func(...)
+            end
+        end
+    end
+
+
     --- Emit a notification signal.
     -- @tparam string name The signal name.
     -- @param ... The signal callback arguments

lib/naughty/core.lua

@@ -18,6 +18,7 @@ local gdebug  = require("gears.debug")
 local screen  = require("awful.screen")
 local gtable  = require("gears.table")
 local gobject = require("gears.object")
+local gsurface = require("gears.surface")
 
 local naughty = {}
 
@@ -274,7 +275,9 @@ function naughty.suspend()
     properties.suspended = true
 end
 
-local conns = gobject._setup_class_signals(naughty)
+local conns = gobject._setup_class_signals(
+    naughty, {allow_chain_of_responsibility=true}
+)
 
 local function resume()
     properties.suspended = false
@@ -561,9 +564,64 @@ naughty.connect_signal("request::screen", naughty.default_screen_handler)
 -- If an icon is found, the handler must set the `icon` property on the `action`
 -- object to a path or a `gears.surface`.
 --
--- @signal request::icon
+-- There is no implementation by default. To use the XDG-icon, the common
+-- implementation will be:
+--
+--    naughty.connect_signal("request::action_icon", function(a, context, hints)
+--         a.icon = menubar.utils.lookup_icon(hints.id)
+--    end)
+--
+-- @signal request::action_icon
 -- @tparam naughty.action action The action.
--- @tparam string icon_name The icon name.
+-- @tparam string context The context.
+-- @tparam table hints
+-- @tparam string args.id The action id. This will often by the (XDG) icon name.
+
+--- Emitted when a notification icon could not be loaded.
+--
+-- When an icon is passed in some "encoded" formats, such as XDG icon names or
+-- network URLs, AwesomeWM will not attempt to load it. If you wish to see the
+-- icon displayed, you must provide an handler. It is highly recommended for
+-- handler to only set `n.icon` when they *found* the icon. That way multiple
+-- handlers can be attached for multiple protocols.
+--
+-- The `context` argument is the origin of the icon to decode. If an handler
+-- only supports one if them, it should check the `context` and return if it
+-- doesn't handle it. The currently valid contexts are:
+--
+-- * app_icon
+-- * clients
+-- * path
+-- * image
+-- * images
+-- * dbus_clear
+--
+-- For example, an implementation which uses the `app_icon` to perform an XDG
+-- icon lookup will look like:
+--
+--    naughty.connect_signal("request::icon", function(n, context, hints)
+--        if context ~= "app_icon" then return end
+--
+--        local path = menubar.utils.lookup_icon(hints.app_icon) or
+--            menubar.utils.lookup_icon(hints.app_icon:lower())
+--
+--        if path then
+--            n.icon = path
+--        end
+--    end)
+--
+-- The `images` context has no handler. It is part of the specification to
+-- handle animations. This is not supported by default.
+--
+-- @signal request::icon
+-- @tparam notification n The notification.
+-- @tparam string context The source of the icon to look for.
+-- @tparam table hints The hints.
+-- @tparam string hints.app_icon The name of the icon to look for.
+-- @tparam string hints.path The path of the icon.
+-- @tparam string hints.image The path or pixmap of the icon.
+-- @see naughty.icon_path_handler
+-- @see naughty.client_icon_handler
 
 --- Emitted when the screen is not defined or being removed.
 -- @signal request::screen
@@ -714,9 +772,49 @@ function naughty.notify(args)
     return nnotif(args)
 end
 
+--- Request handler to get the icon using the clients icons.
+-- @signalhandler naughty.client_icon_handler
+function naughty.client_icon_handler(self, context)
+    if context ~= "clients" then return end
+
+    local clients = self:get_clients()
+
+    for _, t in ipairs { "normal", "dialog" } do
+        for _, c in ipairs(clients) do
+            if c.type == t then
+                self._private.icon = gsurface(c.icon) --TODO support other size
+                return
+            end
+        end
+    end
+end
+
+--- Request handler to get the icon using the image or path.
+-- @signalhandler naughty.icon_path_handler
+function naughty.icon_path_handler(self, context, hints)
+    if context ~= "image" and context ~= "path" then return end
+
+    self._private.icon = gsurface.load_uncached_silently(
+        hints.path or hints.image
+    )
+end
+
+--- Request handler for clearing the icon when asked by ie, DBus.
+-- @signalhandler naughty.icon_clear_handler
+function naughty.icon_clear_handler(self, context, hints) --luacheck: no unused args
+    if context ~= "dbus_clear" then return end
+
+    self._private.icon = nil
+    self:emit_signal("property::icon")
+end
+
 naughty.connect_signal("property::screen"  , update_index)
 naughty.connect_signal("property::position", update_index)
 
+naughty.connect_signal("request::icon", naughty.client_icon_handler)
+naughty.connect_signal("request::icon", naughty.icon_path_handler  )
+naughty.connect_signal("request::icon", naughty.icon_clear_handler )
+
 --@DOC_signals_COMMON@
 
 return setmetatable(naughty, {__index = index_miss, __newindex = set_index_miss})

lib/naughty/dbus.lua

@@ -173,7 +173,7 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters
                 -- and `naughty` doesn't depend on `menubar`, so delegate the
                 -- icon "somewhere" using a request.
                 if hints["action-icons"] and action_id ~= "" then
-                    naughty.emit_signal("request::icon", a, action_id)
+                    naughty.emit_signal("request::action_icon", a, "dbus", {id = action_id})
                 end
 
                 a:connect_signal("invoked", function()
@@ -326,8 +326,12 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters
             -- Update the icon if necessary.
             if app_icon ~= notification._private.app_icon then
                 notification._private.app_icon = app_icon
-                notification._private.icon = nil
+
-                notification:emit_signal("property::icon")
+                naughty._emit_signal_if(
+                    "request::icon", function()
+                        if notification._private.icon then return true end
+                    end, notification, "dbus_clear", {}
+                )
             end
 
             -- Even if no property changed, restart the timeout.

lib/naughty/notification.lua

@@ -18,11 +18,12 @@
 local capi     = { screen = screen }
 local gobject  = require("gears.object")
 local gtable   = require("gears.table")
-local gsurface = require("gears.surface")
 local timer    = require("gears.timer")
+local gfs      = require("gears.filesystem")
 local cst      = require("naughty.constants")
 local naughty  = require("naughty.core")
 local gdebug   = require("gears.debug")
+local pcommon = require("awful.permissions._common")
 
 local notification = {}
 
@@ -647,32 +648,71 @@ for _, prop in ipairs { "category", "resident" } do
     end
 end
 
+-- Stop the request::icon when one is found.
+local function request_filter(self, context, _)
+    if not pcommon.check(self, "notification", "icon", context) then return true end
+    if self._private.icon then return true end
+end
+
+-- Convert encoded local URI to Unix paths.
+local function check_path(input)
+    if type(input) ~= "string" then return nil end
+
+    if input:sub(1,7) == "file://" then
+        input = input:sub(8)
+    end
+
+    -- urldecode
+    input = input:gsub("%%(%x%x)", function(x) return string.char(tonumber(x, 16)) end )
+
+    return gfs.file_readable(input) and input or nil
+end
+
 function notification.get_icon(self)
+    -- Honor all overrides.
     if self._private.icon then
         return self._private.icon == "" and nil or self._private.icon
-    elseif self.image and self.image ~= "" then
-        return self.image
-    elseif self._private.app_icon and self._private.app_icon ~= "" then
-        return self._private.app_icon
     end
 
-    local clients = notification.get_clients(self)
+    -- First, check if the image is passed as a surface or a path.
-
+    if self.image and self.image ~= "" then
-    for _, c in ipairs(clients) do
+        naughty._emit_signal_if("request::icon", request_filter, self, "image", {
-        if c.type == "normal" then
+            image = self.image
-            self._private.icon = gsurface(c.icon)
+        })
-            return self._private.icon
+    elseif self.images then
-        end
+        naughty._emit_signal_if("request::icon", request_filter, self, "images", {
+            images = self.images
+        })
     end
 
-    for _, c in ipairs(clients) do
+    if self._private.icon then
-        if c.type == "dialog" then
+        return self._private.icon == "" and nil or self._private.icon
-            self._private.icon = gsurface(c.icon)
-            return self._private.icon
-        end
     end
 
-    return nil
+    -- Second level of fallback, icon paths.
+    local path = check_path(self._private.app_icon)
+
+    if path then
+        naughty._emit_signal_if("request::icon", request_filter, self, "path", {
+            path = path
+        })
+    end
+
+    if self._private.icon then
+        return self._private.icon == "" and nil or self._private.icon
+    end
+
+    -- Third level fallback is `app_icon`.
+    if self._private.app_icon then
+        naughty._emit_signal_if("request::icon", request_filter, self, "app_icon", {
+            app_icon = self._private.app_icon
+        })
+    end
+
+    -- Finally, the clients.
+    naughty._emit_signal_if("request::icon", request_filter, self, "clients", {})
+
+    return self._private.icon == "" and nil or self._private.icon
 end
 
 function notification.get_clients(self)
@@ -973,6 +1013,22 @@ local function create(args)
     return n
 end
 
+--- Grant a permission for a notification.
+--
+-- @method grant
+-- @tparam string permission The permission name (just the name, no `request::`).
+-- @tparam string context The reason why this permission is requested.
+-- @see awful.permissions
+
+--- Deny a permission for a notification
+--
+-- @method deny
+-- @tparam string permission The permission name (just the name, no `request::`).
+-- @tparam string context The reason why this permission is requested.
+-- @see awful.permissions
+
+pcommon.setup_grant(notification, "notification")
+
 -- This allows notification to be updated later.
 local counter = 1
 

tests/test-naughty-legacy.lua

@@ -4,6 +4,7 @@ local spawn     = require("awful.spawn")
 local naughty   = require("naughty"    )
 local gdebug    = require("gears.debug")
 local gtable    = require("gears.table")
+local gfs       = require("gears.filesystem")
 local cairo     = require("lgi"        ).cairo
 local beautiful = require("beautiful")
 local Gio       = require("lgi"        ).Gio
@@ -886,16 +887,71 @@ table.insert(steps, function()
     return true
 end)
 
+-- Check that request::icon stops when an icon is found.
+table.insert(steps, function()
+
+    local called = 0
+
+    local function set_icon1()
+        called = called + 1
+    end
+
+    local function set_icon2(n)
+        called = called + 1
+        n.icon = big_icon
+    end
+
+    local function set_icon3()
+        called = called + 1
+    end
+
+    naughty.connect_signal("request::icon", set_icon1)
+    naughty.connect_signal("request::icon", set_icon2)
+    naughty.connect_signal("request::icon", set_icon3)
+
+    assert(called == 0)
+
+    local n1 = naughty.notification {
+        title    = "foo",
+        message  = "bar",
+        app_icon = "baz"
+    }
+
+    assert(called == 2)
+
+    n1:destroy()
+
+    naughty.disconnect_signal("request::icon", set_icon1)
+    naughty.disconnect_signal("request::icon", set_icon2)
+    naughty.disconnect_signal("request::icon", set_icon3)
+
+    -- Check if `disconnect_signal` works.
+    n1 = naughty.notification {
+        title    = "foo",
+        message  = "bar",
+        app_icon = "baz"
+    }
+
+    assert(called == 2)
+
+    n1:destroy()
+
+    return true
+end)
+
 local icon_requests = {}
 
 -- Check if the action icon support is detected.
 table.insert(steps, function()
     assert(#active == 0)
 
-    naughty.connect_signal("request::icon", function(a, icon_name)
+    naughty.connect_signal("request::action_icon", function(a, _, hints)
-        icon_requests[icon_name] = a
+        icon_requests[hints.id] = a
+        a.icon = hints.id == "list-add" and small_icon or big_icon
+    end)
 
-        a.icon = icon_name == "list-add" and small_icon or big_icon
+    naughty.connect_signal("request::icon", function(n, _)
+        icon_requests[n] = true
     end)
 
     local hints = {
@@ -913,6 +969,8 @@ table.insert(steps, function()
 
     local n = active[1]
 
+    assert(icon_requests[n])
+
     assert(n._private.freedesktop_hints)
     assert(n._private.freedesktop_hints["action-icons"] == true)
 
@@ -956,6 +1014,7 @@ table.insert(steps, function()
         title    = "foo",
         message  = "bar",
         timeout  = 25000,
+        app_icon = "baz"
     }
 
     -- Make sure the suspension don't cause errors
@@ -981,6 +1040,7 @@ table.insert(steps, function()
     assert(not naughty.suspended)
 
     -- Replace the text
+    assert(icon_requests[n])
     assert(n.title   == "foo")
     assert(n.message == "bar")
     assert(n.text    == "bar")
@@ -1076,6 +1136,57 @@ table.insert(steps, function()
     return true
 end)
 
+-- Check that the various request::icon work.
+table.insert(steps, function()
+    local gsurface = require("gears.surface")
+
+    local ls = gsurface.load_uncached_silently
+
+    function gsurface.load_uncached_silently(input)
+        return {
+            input      = input,
+            get_height = function() return 1 end,
+            get_width  = function() return 1 end,
+        }
+    end
+
+    local fr, gc = gfs.file_readable, naughty.notification.get_clients
+
+    local mocked_client = {
+        type = "normal",
+        icon = "42",
+    }
+
+    function naughty.notification.get_clients()
+        return {mocked_client}
+    end
+
+    function gfs.file_readable() return true end
+
+    local n = naughty.notification {
+        app_icon = "file:///one%20two"
+    }
+
+    assert(type(n.icon) == "table" and n.icon.input == "/one two")
+
+    n:destroy()
+
+    local n2 = naughty.notification {
+        title = "foo"
+    }
+
+    assert(type(n2.icon) == "table" and n2.icon.input == "42")
+
+    n2:destroy()
+
+    -- Restore the real methods.
+    gsurface.load_uncached_silently  = ls
+    naughty.notification.get_clients = gc
+    gfs.file_readable                = fr
+
+    return true
+end)
+
 -- Make sure the legacy popup is used when the new APIs fails.
 table.insert(steps, function()
     assert(naughty.has_display_handler == true)