awesome icon indicating copy to clipboard operation
awesome copied to clipboard

Clicking notification does not urgent/raise client

Open ppacher opened this issue 3 years ago • 9 comments

This issue has been reported on reddit and seems to be an regression from v4.3. If this is not a regression please just close this one. I tried searching the existing issues but wasn't able to find something related.

Reddit Post

Output of awesome --version:

v4.3 according to the reddit discussion.

How to reproduce the issue:

Click on a notification sent by a client (for example Spotify)

Actual result:

The notification disappears, that's all.

Expected result:

Clicking a notification in v4.3 seems to have raised the sending client and set it to urgent. This does not work with the latest git version anymore.

Note that I didn't have time to verify the behavior in v4.3 yet but the reddit user reports that it worked that way. See link above.

Workaround

Using the following signal handler seems to at least mimic the old behavior:

local naughty = require('naughty')
local cst = require("naughty.constants");

naughty.connect_signal('destroyed', function(n, reason)
    if not n.clients then return end
    if reason == cst.notification_closed_reason.dismissed_by_user then
        for _, cli in ipairs(n.clients) do
            cli.urgent = true
            cli:emit_signal("request::activate", {raise=true})
        end
    end
end)

ppacher avatar Sep 18 '20 08:09 ppacher

Clicking a notification in v4.3 seems to have raised the sending client and set it to urgent.

Not really a question to you, just to... the world in general, but... how the heck did this work?

The only way that I can see is that the client requested focus when its notification was closed. It would do so based on the DBus signal that indicated closure. Is that broken now?

Seems so. Running dbus-monitor and notify-send foo in two different terminals, then clicking on the notification results in no output with version 7a759432d3100ff. The same experiment with [I am not sure what version this is, but I would guess it is whatever was latest on 2019-09-21] results in

signal time=1600444645.813826 sender=:1.5 -> destination=(null destination) serial=18 path=/org/freedesktop/Notifications; interface=org.freedesktop.Notifications; member=NotificationClosed
   uint32 4
   uint32 2

psychon avatar Sep 18 '20 15:09 psychon

Seems like the destroy argument to notifications no longer works, but is still used by naughty.dbus. The following patch seems to make things work again, according to dbus-monitor.

diff --git a/lib/naughty/dbus.lua b/lib/naughty/dbus.lua
index 98b36a7f4..dc3af2ba4 100644
--- a/lib/naughty/dbus.lua
+++ b/lib/naughty/dbus.lua
@@ -341,6 +341,7 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters
             args._unique_sender = sender
 
             notification = nnotif(args)
+            notification:connect_signal("destroyed", function(_, r) args.destroy(r) end)
         end
 
         invocation:return_value(GLib.Variant("(u)", { notification.id }))

Assigning to @Elv13 since I am not sure if the bug is "the destroy argument no longer works" or "naughty.dbus uses the new API incorrectly".

psychon avatar Sep 19 '20 09:09 psychon

This seems to have stopped working on awesome master. The workaround mentioned also does not work, as it appears that the notification object no longer has a clients field Edit: I ran dbus-monitor as described above, and see that it correctly emits an event when the notification is closed. However, clicking on the notification no longer raises the corresponding client. What may be the issue?

anihm136 avatar Jan 16 '22 09:01 anihm136

When was the last time this worked? Because this code has changed very little since the PR was merged. Some notification just don't have clients.

Is this generally regressing or is there a specific client you expect to work and it doesn't?

Elv13 avatar Jan 16 '22 09:01 Elv13

I'm not very sure. It was definitely working in 4.3, and I just added the destroyed listener today, so I have a very small sample size. I started looking into this mainly because it wasn't working for a specific application - running a web application in app mode in Brave browser. However, I did a little more digging and it appears that, when multiple apps are opened (as I have), they all share the same PID and hence a notification from one of them has all of them listed as clients. I have not tried notifications from other applications after adding the listener, so I will update once I have more information

anihm136 avatar Jan 16 '22 09:01 anihm136

Tbh, I mentioned that on the PR that it didn't fix the issue on my test,but didn't go anywhere. 😅

seqizz avatar Jan 16 '22 09:01 seqizz

Alright, it now works for me using the listener given above. Not sure if it was a change in API or if the snippet was wrong, but the second argument of emit_signal should have been the reason. Adding that made it work for most clients. I still have the issue with Brave apps, but that's a different issue altogether IMO Note that it still does not work without the listener, whereas it used to work with no additional configuration in 4.3

anihm136 avatar Jan 16 '22 09:01 anihm136

Can you paste the code which work for you?

Elv13 avatar Jan 16 '22 10:01 Elv13

Sorry for the abrupt disappearance. This is part of my config for notifications -

naughty.connect_signal("destroyed", function(n, reason)
	if not n.clients then return end
	if reason == require(
		"naughty.constants"
	).notification_closed_reason.dismissed_by_user then
		local jumped = false
		for _, c in ipairs(n.clients) do
			c.urgent = true
			if jumped then
				c:activate{
					context = "client.jumpto"
				}
			else
				c:jump_to()
				jumped = true
			end
		end
	end
end)

This isn't perfect, but in case multiple clients are registered for the notification, it jumps to the first one and activates all the others. In practice, this is only an issue with brave browser running sites in app mode, all other applications register a single client for each notification and work as expected

Edit: Note that I have replace c:emit_signal('activate', ...) with c:activate(...)

anihm136 avatar Jan 21 '22 10:01 anihm136

I am back on awesomewm git version. Thanks for the above snippet, it doesn't work as good as release version (on release, the application is somehow aware and e.g. switching to the chat which caused the notification) but at least this brings up the client to the front.

seqizz avatar Oct 26 '22 14:10 seqizz

Bumping this because it's a major annoyance. Using discord and the workaround posted here, notifications jump to the application, but don't switch to the target chat. Works fine in release (awesome 4.3), still currently broken as of awesome v4.3-1602-g485661b70.

ProtogenDelta avatar Jun 05 '23 01:06 ProtogenDelta

With https://github.com/awesomeWM/awesome/pull/3863, I can jump to the correct client with:

naughty.connect_signal("destroyed", function(n, reason)
    if not n.clients then
        return
    end
    if reason == cst.notification_closed_reason.dismissed_by_user then
        -- If we clicked on a notification, we get a new urgent client to jump to
        client.connect_signal("property::urgent", function(c)
            -- We don't use notification_client because it's not reliable (Ex: If we have two different instances of chrome)
            -- cf: https://awesomewm.org/apidoc/core_components/naughty.notification.html#clients
            -- So we just check if the client name of our notification is the same as the last urgent client
            -- and jump to this one.
            for _, notification_client in ipairs(n.clients) do
                if c.name == notification_client.name then
                    c:jump_to()
                end
            end
        end)
    end
end)

BarbUk avatar Oct 09 '23 15:10 BarbUk

@BarbUk thanks. This looks like it is jumping to the correct place (at least on slack) but since slack interface is a lot weirder, I am not sure yet. Will try to test this properly when I have time.

seqizz avatar Oct 09 '23 16:10 seqizz