Merge pull request #3443 from Elv13/fix_3396

Improve suspending the notifications

lib/naughty/core.lua

@@ -292,14 +292,20 @@ local conns = gobject._setup_class_signals(
 
 local function resume()
     properties.suspended = false
-    for _, v in pairs(naughty.notifications.suspended) do
+    for _, v in ipairs(naughty.notifications.suspended) do
         local args = v._private.args
         assert(args)
         v._private.args = nil
 
+        v:emit_signal("property::suspended", false)
+
         naughty.emit_signal("added", v, args)
         naughty.emit_signal("request::display", v, "resume", args)
         if v.timer then v.timer:start() end
+
+        if not v._private.args then
+            v._private.args = args
+        end
     end
     naughty.notifications.suspended = { }
 end
@@ -394,8 +400,8 @@ end
 -- @staticfct naughty.get_by_id
 function naughty.get_by_id(id)
     -- iterate the notifications to get the notfications with the correct ID
-    for s in pairs(naughty.notifications) do
+    for s in capi.screen do
-        for p in pairs(naughty.notifications[s]) do
+        for p in pairs(naughty.notifications[s] or {}) do
             for _, notification in pairs(naughty.notifications[s][p]) do
                 if notification.id == id then
                     return notification
@@ -465,7 +471,7 @@ local function cleanup(self, reason)
     assert(reason, "Use n:destroy() instead of emitting the signal directly")
 
     if properties.suspended then
-        for k, v in pairs(naughty.notifications.suspended) do
+        for k, v in ipairs(naughty.notifications.suspended) do
             if v == self then
                 table.remove(naughty.notifications.suspended, k)
                 break
@@ -507,6 +513,25 @@ local function get_suspended(self)
     return properties.suspended and not self.ignore_suspend
 end
 
+function naughty.set_suspended(value)
+    if properties["suspended"] == value then return end
+
+    properties["suspended"] = value
+
+    if value then
+        for _, n in pairs(naughty._active) do
+            if n.timer and n.timer.started then
+                n.timer:stop()
+            end
+
+            n:emit_signal("property::suspended", true)
+            table.insert(naughty.notifications.suspended, n)
+        end
+    else
+        resume()
+    end
+end
+
 function naughty.set_expiration_paused(p)
     properties.expiration_paused = p
 
@@ -669,8 +694,9 @@ local function register(notification, args)
 
     notification._private.registered = true
 
-    if properties.suspended and not args.ignore_suspend then
     notification._private.args = args
+
+    if properties.suspended and not args.ignore_suspend then
         table.insert(naughty.notifications.suspended, notification)
     else
         naughty.emit_signal("added", notification, args)
@@ -702,9 +728,6 @@ local function set_index_miss(_, key, value)
     elseif properties[key] ~= nil then
         assert(type(value) == "boolean")
         properties[key] = value
-        if not value then
-            resume()
-        end
 
         naughty.emit_signal("property::"..key, value)
     else

lib/naughty/layout/box.lua

@@ -46,6 +46,19 @@ local function init_screen(s)
     return by_position[s]
 end
 
+local function disconnect(self)
+    if self._private.notification then
+        self._private.notification:disconnect_signal("destroyed",
+            self._private.destroy_callback)
+
+        self._private.notification:disconnect_signal("property::margin",
+            self._private.update)
+
+        self._private.notification:disconnect_signal("property::suspended",
+            self._private.hide)
+    end
+end
+
 ascreen.connect_for_each_screen(init_screen)
 
 -- Manually cleanup to help the GC.
@@ -95,8 +108,6 @@ local function update_position(position, preset)
             -- The first entry is aligned to the workarea, then the following to the
             -- previous widget.
             placement[k==1 and position:gsub("_middle", "") or "next_to"](wdg, args)
-
-            wdg.visible = true
         end
     end
 end
@@ -118,6 +129,8 @@ local function finish(self)
     end
 
     update_position(self.position, preset)
+
+    disconnect(self)
 end
 
 -- It isn't a good idea to use the `attach` `awful.placement` property. If the
@@ -264,23 +277,27 @@ local function init(self, notification)
 
     table.insert(init_screen(s)[position], self)
 
-    local function update() update_position(position, preset) end
+    self._private.update = function() update_position(position, preset) end
+    self._private.hide = function(_, value)
+        if value then
+            finish(self)
+        end
+    end
 
-    self:connect_signal("property::geometry", update)
+    self:connect_signal("property::geometry", self._private.update)
-    notification:connect_signal("property::margin", update)
+    notification:weak_connect_signal("property::margin", self._private.update)
-    notification:connect_signal("destroyed", self._private.destroy_callback)
+    notification:weak_connect_signal("property::suspended", self._private.hide)
+    notification:weak_connect_signal("destroyed", self._private.destroy_callback)
 
     update_position(position, preset)
 
+    self.visible = true
 end
 
 function box:set_notification(notif)
     if self._private.notification == notif then return end
 
-    if self._private.notification then
+    disconnect(self)
-        self._private.notification:disconnect_signal("destroyed",
-            self._private.destroy_callback)
-    end
 
     init(self, notif)
 

lib/naughty/notification.lua

@@ -1033,7 +1033,7 @@ local function create(args)
     end
 
     -- Let all listeners handle the actual visual aspects
-    if (not n.ignore) and ((not n.preset) or n.preset.ignore ~= true) then
+    if (not n.ignore) and ((not n.preset) or n.preset.ignore ~= true) and (not naughty.suspended) then
         naughty.emit_signal("request::display" , n, "new", args)
         naughty.emit_signal("request::fallback", n, "new", args)
     end

tests/test-naughty-legacy.lua

@@ -1074,6 +1074,7 @@ table.insert(steps, function()
     assert(naughty.get_by_id(id) == n)
     assert(naughty.getById(42) ~= n)
     assert(naughty.get_by_id(42) ~= n)
+    assert(naughty.get_by_id(9999) == nil)
 
     -- The timeout
     local real, called = n.reset_timeout, false

tests/test-naughty-suspended.lua

@@ -0,0 +1,106 @@
+local naughty = require("naughty")
+local notification = require("naughty.notification")
+
+local steps = {}
+
+-- Hijack the naughty.layout.box to track them.
+local real_box = getmetatable(naughty.layout.box).__call
+local boxes = {}
+
+getmetatable(naughty.layout.box).__call = function(_, args)
+    local ret = real_box(_, args)
+
+    table.insert(boxes, ret)
+
+    return ret
+end
+
+naughty.suspended = true
+
+local display_count, notifs = 0, setmetatable({}, {__mode = "v"})
+
+naughty.connect_signal("request::display", function()
+    display_count = display_count + 1
+end)
+
+table.insert(steps, function()
+    notifs[1] = notification {
+        title = "test1"
+    }
+
+    notifs[2] = notification {
+        title = "test2"
+    }
+
+    assert(display_count == 0)
+    assert((not notifs[1].timer) or not notifs[1].timer.started)
+    assert((not notifs[2].timer) or not notifs[2].timer.started)
+
+    return true
+end)
+
+table.insert(steps, function()
+    assert(display_count == 0)
+
+    naughty.suspended = false
+
+    assert(display_count == 2)
+    assert(#boxes == 2)
+    assert(notifs[1].timer.started)
+    assert(notifs[2].timer.started)
+
+    notifs[3] = notification {
+        title = "test2"
+    }
+
+    naughty.suspended = true
+
+    assert(display_count == 3)
+
+    return true
+end)
+
+table.insert(steps, function()
+    assert(display_count == 3)
+    assert(not boxes[1].visible)
+    assert(not boxes[2].visible)
+    assert((not notifs[1].timer) or not notifs[1].timer.started)
+    assert((not notifs[2].timer) or not notifs[2].timer.started)
+    assert((not notifs[3].timer) or not notifs[3].timer.started)
+
+    naughty.suspended = false
+
+    return true
+end)
+
+table.insert(steps, function()
+    assert(display_count == 6)
+    assert(not boxes[1].visible)
+    assert(not boxes[2].visible)
+    assert(boxes[4].visible)
+    assert(boxes[5].visible)
+    assert(boxes[6].visible)
+
+    assert(notifs[1].timer.started)
+    assert(notifs[2].timer.started)
+    assert(notifs[3].timer.started)
+
+    setmetatable(boxes, {__mode = "v"})
+
+    notifs[1]:destroy()
+    notifs[2]:destroy()
+    notifs[3]:destroy()
+
+    return true
+end)
+
+table.insert(steps, function()
+    collectgarbage("collect")
+
+    if #notifs > 0 then return end
+    if #boxes > 0 then return end
+
+    return true
+end)
+
+require("_runner").run_steps(steps)