Merge pull request #2715 from Elv13/notif_replace_id2

Fix 4 issues related to the notification refactoring

lib/naughty/core.lua

@@ -360,6 +360,8 @@ end
 
 -- Remove the notification from the internal list(s)
 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
             if v == self then
@@ -553,6 +555,17 @@ function naughty.notify(args)
     --TODO v6 remove this hack
     nnotif = nnotif or require("naughty.notification")
 
+    -- The existing notification object, if any.
+    local n = args and args.replaces_id and
+        naughty.get_by_id(args.replaces_id) or nil
+
+    -- It was possible to update the notification content using `replaces_id`.
+    -- This is a concept that come from the dbus API and leaked into the public
+    -- API. It has all kind of issues and brokenness, but it being used.
+    if n then
+        return gtable.crush(n, args)
+    end
+
     return nnotif(args)
 end
 

lib/naughty/notification.lua

@@ -249,6 +249,8 @@ function notification:destroy(reason, keep_visible)
           return false
     end
 
+    reason = reason or cst.notification_closed_reason.dismissed_by_user
+
     self:emit_signal("destroyed", reason, keep_visible)
 
     self._private.is_destroyed = true

tests/test-naughty-legacy.lua

@@ -92,21 +92,45 @@ local function destroyed_callback(n, reason)
     table.insert(destroyed, n)
 end
 
-naughty.connect_signal("destroyed", destroyed_callback)
+-- Test vertical overlapping
+local function test_overlap()
+    local wa = mouse.screen.workarea
 
+    for _, n1 in ipairs(active) do
+        assert(n1.box)
+        assert(n1.id)
+
+        -- Check for overlapping the workarea
+        assert(n1.box.y+default_height < wa.y+wa.height)
+        assert(n1.box.y >= wa.y)
+
+        -- Check for overlapping each other
+        for _, n2 in ipairs(active) do
+            assert(n2.box)
+            if n1 ~= n2 then
+                local geo1, geo2 = n1.box:geometry(), n2.box:geometry()
+                assert(geo1.height == geo2.height)
+                assert(geo1.height + 2*n1.box.border_width + naughty.config.spacing
+                    == default_height)
+
+                if n1.position == n2.position then
+                    assert(
+                        geo1.y >= geo2.y+default_height or
+                        geo2.y >= geo1.y+default_height
+                    )
+                end
+            end
+        end
+    end
+end
+
+-- Set the default size.
 table.insert(steps, function()
-    if not has_cmd_notify then return true end
 
-    spawn{ 'notify-send', 'title', 'message', '-t', '25000' }
-
-    return true
-end)
-
-table.insert(steps, function()
-    if not has_cmd_notify then return true end
-    if #active ~= 1 then return end
-
-    local n = active[1]
+    local n = naughty.notification {
+        title   = "title",
+        message = "message"
+    }
 
     assert(n.box)
     local offset = 2*n.box.border_width
@@ -116,6 +140,29 @@ table.insert(steps, function()
     assert(default_width  > 0)
     assert(default_height > 0)
 
+    n:destroy()
+
+    -- This one doesn't count.
+    active, destroyed, reasons, counter = {}, {}, {}, 0
+    return true
+end)
+
+naughty.connect_signal("destroyed", destroyed_callback)
+
+if has_cmd_notify then
+    table.insert(steps, function()
+        spawn{ 'notify-send', 'title', 'message', '-t', '25000' }
+
+        return true
+    end)
+
+    table.insert(steps, function()
+        if #active ~= 1 then return end
+
+        local n = active[1]
+
+        assert(n.box)
+
         -- Make sure the expiration timer is started
         assert(n.timer)
         assert(n.timer.started)
@@ -251,38 +298,6 @@ table.insert(steps, function()
         return true
     end)
 
--- Test vertical overlapping
-local function test_overlap()
-    local wa = mouse.screen.workarea
-
-    for _, n1 in ipairs(active) do
-        assert(n1.box)
-        assert(n1.id)
-
-        -- Check for overlapping the workarea
-        assert(n1.box.y+default_height < wa.y+wa.height)
-        assert(n1.box.y >= wa.y)
-
-        -- Check for overlapping each other
-        for _, n2 in ipairs(active) do
-            assert(n2.box)
-            if n1 ~= n2 then
-                local geo1, geo2 = n1.box:geometry(), n2.box:geometry()
-                assert(geo1.height == geo2.height)
-                assert(geo1.height + 2*n1.box.border_width + naughty.config.spacing
-                    == default_height)
-
-                if n1.position == n2.position then
-                    assert(
-                        geo1.y >= geo2.y+default_height or
-                        geo2.y >= geo1.y+default_height
-                    )
-                end
-            end
-        end
-    end
-end
-
     -- Check the lack of overlapping and the presence of the expected content.
     table.insert(steps, function()
         local wa = mouse.screen.workarea
@@ -322,6 +337,8 @@ table.insert(steps, function()
         return true
     end)
 
+end
+
 local positions = {
     "top_left"      , "top_middle"    , "top_right"     ,
     "bottom_left"   , "bottom_middle" , "bottom_right"  ,
@@ -748,10 +765,10 @@ table.insert(steps, function()
     assert(n.width > width)
     assert(n.height == height)
     width, height = n.width, n.height
-    naughty.replace_text(n, "foo", "bar\nbaz")
+    naughty.replace_text(n, "foo", "bar\nbar")
     assert(n.title   == "foo")
-    assert(n.message == "bar\nbaz")
-    assert(n.text    == "bar\nbaz")
+    assert(n.message == "bar\nbar")
+    assert(n.text    == "bar\nbar")
     assert(n.width < width)
     assert(n.height > height)
     width, height = n.width, n.height
@@ -789,7 +806,21 @@ table.insert(steps, function()
     naughty.suspended = false
 
     -- The old notify function and "text" instead of "message"
-    naughty.notify { text = "foo" }
+    n = naughty.notify { text = "foo" }
+    assert(n.message == "foo")
+    assert(n.text    == "foo")
+
+    -- Calling `naughty.notify` with replace_id.
+    n2 = naughty.notify {
+        replaces_id = n.id,
+        message     = "bar",
+        title       = "foo",
+    }
+
+    assert(n         == n2   )
+    assert(n.message == "bar")
+    assert(n.text    == "bar")
+    assert(n.title   == "foo")
 
     -- Finish by testing disconnect_signal
     naughty.disconnect_signal("destroyed", destroyed_callback)