mate-desktop / mate-notification-daemon

Daemon to display passive pop-up notifications
https://mate-desktop.org
GNU General Public License v2.0
30 stars 26 forks source link

daemon: Properly update the set of monitors when it changes #226

Closed cwendling closed 1 month ago

cwendling commented 1 month ago

May fix #218, or part of it.

Beware the I didn't test this myself, but @joakim-tjernlund did. We need more testing, and see whether this effectively fixes the issues as reproduced by @raveit65 and @lukefromdc, as the fix here is related to monitors changing, while their reproducers might not have been.

The fix here is for properly updating the list of available monitors when that set changes. Before this patch, only the last monitor(s) would be removed, rather than the one(s) that got removed, which should cause issues if the primary monitor is removed, or more generally if any but the last is removed. While having this issue unnoticed could be a bit surprising, I suppose it is fairly unusual to disconnect the primary monitor, and was fairly unusual as well to have more than 2.

lukefromdc commented 1 month ago

We have this travis error in Federa which is NOT affected by this PR but is blocking all new releases. I don't know how to fix it as I have zero Ruby experience:

Install deployment dependencies

You have already activated uri 0.13.1, but your Gemfile requires uri 0.13.0. Since uri is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports uri as a default gem. (Gem::LoadError)

failed to deploy
lukefromdc commented 1 month ago

Hoping for another review

lukefromdc commented 1 month ago

If someone could either fix the gem issue with Travis Fedora builds or tell me how to do it, I could get this out the door with a new release. Note that translations appear to still be broken with at least one new language, that too is not something I know how to fix. Thus a bugfix release would have to use the old translations