Fix the broken test due to the "soft" merge conflict of two notification pull requests. (#2751)

* naughty.legacy: Fix a regression caused by a prior fix.

The title was only set "later" because it was called too early.

The intended result was to prevent the code from being executed when
there is no leagcy popup, but it had this side effect.

* naughty.dbus: Expose the new "private" methods so they can be tested.

Because it now uses Gio instead of capi.dbus, it isn't possible to
just shim the backend anymore.

* shims: Upgrade the dbus shims to also emulate some Gio behavior.

As usual, it is the most basic version that produces the correct
result. It doesn't try to comply to the real API.

lib/naughty/dbus.lua

@@ -120,151 +120,168 @@ local function convert_icon(w, h, rowstride, channels, data)
     return res
 end
 
-local function protected_method_call(_, sender, object_path, interface, method, parameters, invocation)
+local notif_methods = {}
-    if method == "Notify" then
+
-        local appname, replaces_id, icon, title, text, actions, hints, expire =
+function notif_methods.Notify(sender, object_path, interface, method, parameters, invocation)
-            unpack(parameters.value)
+    local appname, replaces_id, icon, title, text, actions, hints, expire =
-        local args = {}
+        unpack(parameters.value)
-        if text ~= "" then
+
-            args.message = text
+    local args = {}
-            if title ~= "" then
+    if text ~= "" then
-                args.title = title
+        args.message = text
-            end
+        if title ~= "" then
+            args.title = title
+        end
+    else
+        if title ~= "" then
+            args.message = title
         else
-            if title ~= "" then
+            -- FIXME: We have to reply *something* to the DBus invocation.
-                args.message = title
+            -- Right now this leads to a memory leak, I think.
-            else
-                -- FIXME: We have to reply *something* to the DBus invocation.
-                -- Right now this leads to a memory leak, I think.
-                return
-            end
-        end
-        if appname ~= "" then
-            args.appname = appname
-        end
-        for _, obj in pairs(dbus.config.mapping) do
-            local filter, preset = obj[1], obj[2]
-            if (not filter.urgency or filter.urgency == hints.urgency) and
-            (not filter.category or filter.category == hints.category) and
-            (not filter.appname or filter.appname == appname) then
-                args.preset = gtable.join(args.preset, preset)
-            end
-        end
-        local preset = args.preset or cst.config.defaults
-        local notification
-        if actions then
-            args.actions = {}
-
-            for i = 1,#actions,2 do
-                local action_id = actions[i]
-                local action_text = actions[i + 1]
-
-                if action_id == "default" then
-                    args.run = function()
-                        sendActionInvoked(notification.id, "default")
-                        notification:destroy(cst.notification_closed_reason.dismissed_by_user)
-                    end
-                elseif action_id ~= nil and action_text ~= nil then
-                    local a = naction {
-                        name     = action_text,
-                        position = action_id,
-                    }
-
-                    a:connect_signal("invoked", function()
-                        sendActionInvoked(notification.id, action_id)
-                        notification:destroy(cst.notification_closed_reason.dismissed_by_user)
-                    end)
-
-                    table.insert(args.actions, a)
-                end
-            end
-        end
-        args.destroy = function(reason)
-            sendNotificationClosed(notification.id, reason)
-        end
-        local legacy_data = { -- This data used to be generated by AwesomeWM's C code
-            type = "method_call", interface = interface, path = object_path,
-            member = method, sender = sender, bus = "session"
-        }
-        if not preset.callback or (type(preset.callback) == "function" and
-            preset.callback(legacy_data, appname, replaces_id, icon, title, text, actions, hints, expire)) then
-            if icon ~= "" then
-                args.icon = icon
-            elseif hints.icon_data or hints.image_data then
-                -- Icon data is a bit complex and hence needs special care:
-                -- .value breaks with the array of bytes (ay) that we get here.
-                -- So, bypass it and look up the needed value differently
-                local icon_data
-                for k, v in parameters:get_child_value(7 - 1):pairs() do
-                    if k == "icon_data" then
-                        icon_data = v
-                    elseif k == "image_data" and icon_data == nil then
-                        icon_data = v
-                    end
-                end
-
-                -- icon_data is an array:
-                -- 1 -> width
-                -- 2 -> height
-                -- 3 -> rowstride
-                -- 4 -> has alpha
-                -- 5 -> bits per sample
-                -- 6 -> channels
-                -- 7 -> data
-
-                -- Get the value as a GVariant and then use LGI's special
-                -- GVariant.data to get that as an LGI byte buffer. That one can
-                -- then by converted to a string via its __tostring metamethod.
-                local data = tostring(icon_data:get_child_value(7 - 1).data)
-                args.icon = convert_icon(icon_data[1], icon_data[2], icon_data[3], icon_data[6], data)
-            end
-            if replaces_id and replaces_id ~= "" and replaces_id ~= 0 then
-                args.replaces_id = replaces_id
-            end
-            if expire and expire > -1 then
-                args.timeout = expire / 1000
-            end
-            args.freedesktop_hints = hints
-
-            -- Try to update existing objects when possible
-            notification = naughty.get_by_id(replaces_id)
-
-            if notification then
-                for k, v in pairs(args) do
-                    if k == "destroy" then k = "destroy_cb" end
-                    notification[k] = v
-                end
-            else
-                notification = nnotif(args)
-            end
-
-            invocation:return_value(GLib.Variant("(u)", { notification.id }))
             return
         end
-
-        invocation:return_value(GLib.Variant("(u)", { nnotif._gen_next_id() }))
-    elseif method == "CloseNotification" then
-        local obj = naughty.get_by_id(parameters.value[1])
-        if obj then
-            obj:destroy(cst.notification_closed_reason.dismissed_by_command)
-        end
-        invocation:return_value(GLib.Variant("()"))
-    elseif method == "GetServerInfo" or method == "GetServerInformation" then
-        -- name of notification app, name of vender, version, specification version
-        invocation:return_value(GLib.Variant("(ssss)", {
-            "naughty", "awesome", capi.awesome.version, "1.0"
-        }))
-    elseif method == "GetCapabilities" then
-        -- We actually do display the body of the message, we support <b>, <i>
-        -- and <u> in the body and we handle static (non-animated) icons.
-        invocation:return_value(GLib.Variant("(as)", {
-            { "body", "body-markup", "icon-static", "actions" }
-        }))
     end
+    if appname ~= "" then
+        args.appname = appname
+    end
+    for _, obj in pairs(dbus.config.mapping) do
+        local filter, preset = obj[1], obj[2]
+        if (not filter.urgency or filter.urgency == hints.urgency) and
+        (not filter.category or filter.category == hints.category) and
+        (not filter.appname or filter.appname == appname) then
+            args.preset = gtable.join(args.preset, preset)
+        end
+    end
+    local preset = args.preset or cst.config.defaults
+    local notification
+    if actions then
+        args.actions = {}
+
+        for i = 1,#actions,2 do
+            local action_id = actions[i]
+            local action_text = actions[i + 1]
+
+            if action_id == "default" then
+                args.run = function()
+                    sendActionInvoked(notification.id, "default")
+                    notification:destroy(cst.notification_closed_reason.dismissed_by_user)
+                end
+            elseif action_id ~= nil and action_text ~= nil then
+                local a = naction {
+                    name     = action_text,
+                    position = action_id,
+                }
+
+                a:connect_signal("invoked", function()
+                    sendActionInvoked(notification.id, action_id)
+                    notification:destroy(cst.notification_closed_reason.dismissed_by_user)
+                end)
+
+                table.insert(args.actions, a)
+            end
+        end
+    end
+    args.destroy = function(reason)
+        sendNotificationClosed(notification.id, reason)
+    end
+    local legacy_data = { -- This data used to be generated by AwesomeWM's C code
+        type = "method_call", interface = interface, path = object_path,
+        member = method, sender = sender, bus = "session"
+    }
+    if not preset.callback or (type(preset.callback) == "function" and
+        preset.callback(legacy_data, appname, replaces_id, icon, title, text, actions, hints, expire)) then
+        if icon ~= "" then
+            args.icon = icon
+        elseif hints.icon_data or hints.image_data then
+            -- Icon data is a bit complex and hence needs special care:
+            -- .value breaks with the array of bytes (ay) that we get here.
+            -- So, bypass it and look up the needed value differently
+            local icon_data
+            for k, v in parameters:get_child_value(7 - 1):pairs() do
+                if k == "icon_data" then
+                    icon_data = v
+                elseif k == "image_data" and icon_data == nil then
+                    icon_data = v
+                end
+            end
+
+            -- icon_data is an array:
+            -- 1 -> width
+            -- 2 -> height
+            -- 3 -> rowstride
+            -- 4 -> has alpha
+            -- 5 -> bits per sample
+            -- 6 -> channels
+            -- 7 -> data
+
+            -- Get the value as a GVariant and then use LGI's special
+            -- GVariant.data to get that as an LGI byte buffer. That one can
+            -- then by converted to a string via its __tostring metamethod.
+            local data = tostring(icon_data:get_child_value(7 - 1).data)
+            args.icon = convert_icon(icon_data[1], icon_data[2], icon_data[3], icon_data[6], data)
+        end
+        if replaces_id and replaces_id ~= "" and replaces_id ~= 0 then
+            args.replaces_id = replaces_id
+        end
+        if expire and expire > -1 then
+            args.timeout = expire / 1000
+        end
+        args.freedesktop_hints = hints
+
+        -- Try to update existing objects when possible
+        notification = naughty.get_by_id(replaces_id)
+
+        if notification then
+            for k, v in pairs(args) do
+                if k == "destroy" then k = "destroy_cb" end
+                notification[k] = v
+            end
+        else
+            notification = nnotif(args)
+        end
+
+        invocation:return_value(GLib.Variant("(u)", { notification.id }))
+        return
+    end
+
+    invocation:return_value(GLib.Variant("(u)", { nnotif._gen_next_id() }))
 end
 
-local function method_call(...)
+function notif_methods.CloseNotification(_, _, _, _, parameters, invocation)
-    protected_call(protected_method_call, ...)
+    local obj = naughty.get_by_id(parameters.value[1])
+    if obj then
+        obj:destroy(cst.notification_closed_reason.dismissed_by_command)
+    end
+    invocation:return_value(GLib.Variant("()"))
+end
+
+function notif_methods.GetServerInformation(_, _, _, _, _, invocation)
+    -- name of notification app, name of vender, version, specification version
+    invocation:return_value(GLib.Variant("(ssss)", {
+        "naughty", "awesome", capi.awesome.version, "1.0"
+    }))
+end
+
+function notif_methods.GetCapabilities(_, _, _, _, _, invocation)
+    -- We actually do display the body of the message, we support <b>, <i>
+    -- and <u> in the body and we handle static (non-animated) icons.
+    invocation:return_value(GLib.Variant("(as)", {
+        { "body", "body-markup", "icon-static", "actions" }
+    }))
+end
+
+local function method_call(_, sender, object_path, interface, method, parameters, invocation)
+    if not notif_methods[method] then return end
+
+    protected_call(
+        notif_methods[method],
+        sender,
+        object_path,
+        interface,
+        method,
+        parameters,
+        invocation
+    )
 end
 
 local function on_bus_acquire(conn, _)
@@ -322,6 +339,9 @@ Gio.bus_own_name(Gio.BusType.SESSION, "org.freedesktop.Notifications",
     Gio.BusNameOwnerFlags.NONE, GObject.Closure(on_bus_acquire),
     GObject.Closure(on_name_acquired), GObject.Closure(on_name_lost))
 
+-- For testing
+dbus._notif_methods = notif_methods
+
 return dbus
 
 -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80

lib/naughty/layout/legacy.lua

@@ -211,6 +211,8 @@ local escape_subs    = { ['<'] = "&lt;", ['>'] = "&gt;", ['&'] = "&amp;" }
 
 -- Cache the markup
 local function set_escaped_text(self)
+    if not self.box then return end
+
     local text = self.message or ""
     local title = self.title and self.title .. "\n" or ""
 
@@ -370,7 +372,6 @@ function naughty.default_notification_handler(notification, args)
     textbox:set_font(font)
 
     notification.textbox = textbox
-    set_escaped_text(notification)
 
     -- Update the content if it changes
     notification:connect_signal("property::message", set_escaped_text)
@@ -513,10 +514,14 @@ function naughty.default_notification_handler(notification, args)
     notification.box.visible = true
 
     -- populate widgets
+    set_escaped_text(notification)
+
     local layout = wibox.layout.fixed.horizontal()
+
     if iconmargin then
         layout:add(iconmargin)
     end
+
     layout:add(marginbox)
 
     local completelayout = wibox.layout.fixed.vertical()

tests/examples/shims/dbus.lua

@@ -1,6 +1,40 @@
+local lgi = require("lgi")
+local Gio = lgi.Gio
+
 local dbus = awesome._shim_fake_class()
 
-function dbus.notify_send(...)
+-- Monkeypatch away the real dbus support
+Gio.bus_own_name = function() end
+
+--HACK it used to be an internal API, which made testing easy, but now it uses
+-- GDBus, so this shims a small subset of its API and use some internal APIs
+-- to access "private" methods. Note that it would be cleaner to implement an
+-- high level (and testable) binding and use signals again.
+local ndbus = nil
+
+local invocation = {
+    return_value = function() end
+}
+
+local function parameters_miss(t, k)
+    if k == "get_child_value" then
+        return function(idx) return t[idx-1] end
+    end
+end
+
+function dbus.notify_send(_--[[appdata]], ...)
+    ndbus = ndbus or require("naughty.dbus")
+
+    ndbus._notif_methods.Notify(
+        "sender",
+        "/org/freedesktop/Notifications",
+        "org.freedesktop.Notifications",
+        "Notify",
+        setmetatable({value={...}}, {__index=parameters_miss}),
+        invocation
+    )
+
+    -- Legacy API
     dbus.emit_signal("org.freedesktop.Notifications", ...)
 end