diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,23 @@ +# 7.3.1 + +## Fixes + +* Fix `unsubscribe` removing every X11 event listener *except* the one it was + asked to remove. Unsubscribed listeners kept firing while live ones were + dropped, which is why the workspaces widget could stop updating after a + display configuration change (#465). Thanks to @sgrb (#694). +* Stop `autoSizeImage` (the `ImageResize` scaling strategy and + `imageMenuItemNew`) from growing without bound. Padding and border are now + read at allocation time instead of once at construction, and an allocation + that grows as a direct result of the pixbuf the widget just set is no longer + answered with a larger pixbuf. Previously a stale border reading made every + `size-allocate` request a slightly larger image, which pegged the CPU and + leaked memory until the process was killed. +* Require `xdg-desktop-entry` with the fixed parser, so desktop files with + localised keys such as `GenericName[de]` resolve again. Before this, the + MPRIS2 player icon (and any other desktop-entry based icon lookup) fell back + to the default icon for most real-world desktop files. + # 7.3.0 ## Breaking changes diff --git a/src/System/Taffybar/Context.hs b/src/System/Taffybar/Context.hs --- a/src/System/Taffybar/Context.hs +++ b/src/System/Taffybar/Context.hs @@ -610,10 +610,16 @@ Nothing -> return Nothing Just activeWindow -> do display <- getDisplay - (_, x, y, width, height, _, _) <- lift $ safeGetGeometry display activeWindow - let centerX = fromIntegral x + fromIntegral width `div` 2 - centerY = fromIntegral y + fromIntegral height `div` 2 - return $ Just (centerX, centerY) + maybeGeometry <- + lift $ + (Just <$> safeGetGeometry display activeWindow) + `catchAny` const (return Nothing) + case maybeGeometry of + Nothing -> return Nothing + Just (_, x, y, width, height, _, _) -> do + let centerX = fromIntegral x + fromIntegral width `div` 2 + centerY = fromIntegral y + fromIntegral height `div` 2 + return $ Just (centerX, centerY) where guardElem value values = if value `elem` values @@ -1050,7 +1056,7 @@ unsubscribe :: Unique -> Taffy IO () unsubscribe identifier = do listenersVar <- asks listeners - lift $ MV.modifyMVar_ listenersVar $ return . filter ((== identifier) . fst) + lift $ MV.modifyMVar_ listenersVar $ return . filter ((/= identifier) . fst) -- | Subscribe to all incoming events on the X11 event loop. The returned -- "Unique" value can be used to unregister the listener using "unsuscribe". diff --git a/src/System/Taffybar/Widget/Generic/AutoSizeImage.hs b/src/System/Taffybar/Widget/Generic/AutoSizeImage.hs --- a/src/System/Taffybar/Widget/Generic/AutoSizeImage.hs +++ b/src/System/Taffybar/Widget/Generic/AutoSizeImage.hs @@ -72,13 +72,16 @@ _ <- widgetSetClassGI image "auto-size-image" lastAllocation <- MV.newMVar 0 - -- XXX: Gtk seems to report information about padding etc inconsistently, - -- which is why we look it up once, at startup. This means that we won't - -- properly react to changes to these values, which could be a pretty nasty - -- gotcha for someone down the line. :( - borderInfo <- getBorderInfo image + -- Set just before we swap the pixbuf, so the size-allocate GTK runs in + -- response can be told apart from one the parent initiated. + selfAllocation <- MV.newMVar False let setPixbuf force allocation = do + -- Padding and border are read at allocation time. Before the widget + -- is parented and styled they come back as zero, and a stale value + -- here makes every pixbuf we set request a larger allocation than + -- the one it was scaled for. + borderInfo <- getBorderInfo image _width <- Gdk.getRectangleWidth allocation _height <- Gdk.getRectangleHeight allocation @@ -90,10 +93,24 @@ _ -> width previousSize <- MV.readMVar lastAllocation + selfTriggered <- MV.swapMVar selfAllocation False - when (size /= previousSize || force) $ do - MV.modifyMVar_ lastAllocation $ const $ return size + -- The allocation grew as a direct result of the pixbuf we just set, + -- so the widget has chrome that borderInfo does not account for. + -- Loading a pixbuf for the new size would repeat the growth forever. + let runaway = not force && selfTriggered && size > previousSize + when runaway $ do + void $ MV.swapMVar lastAllocation size + imageLog DEBUG $ + printf + "Ignoring self-triggered growth of auto-size image from %s to %s" + (show previousSize) + (show size) + + when ((size /= previousSize && not runaway) || force) $ do + void $ MV.swapMVar lastAllocation size + pixbuf <- getPixbuf size pbWidth <- fromMaybe 0 <$> traverse Gdk.getPixbufWidth pixbuf pbHeight <- fromMaybe 0 <$> traverse Gdk.getPixbufHeight pixbuf @@ -114,6 +131,7 @@ (show pbWidth) (show pbHeight) + void $ MV.swapMVar selfAllocation True Gtk.imageSetFromPixbuf image pixbuf postGUIASync $ Gtk.widgetQueueResize image diff --git a/taffybar.cabal b/taffybar.cabal --- a/taffybar.cabal +++ b/taffybar.cabal @@ -1,6 +1,6 @@ cabal-version: 3.4 name: taffybar -version: 7.3.0 +version: 7.3.1 synopsis: A desktop bar similar to xmobar, but with more GUI description: Taffybar is a desktop status bar with GTK widgets for window manager state, system information, tray icons, and custom user modules. @@ -123,7 +123,7 @@ , tuple >= 0.3.0.2 && < 0.4 , unix >= 2.7 && < 2.9 , utf8-string >= 1.0 && < 1.1 - , xdg-desktop-entry >= 0.1.1.5 && < 0.2 + , xdg-desktop-entry >= 0.1.1.6 && < 0.2 , xdg-basedir >= 0.2 && < 0.3 , xml >= 1.3 && < 1.4 , xml-helpers >= 1.0 && < 1.1 diff --git a/test/unit/System/Taffybar/ContextSpec.hs b/test/unit/System/Taffybar/ContextSpec.hs --- a/test/unit/System/Taffybar/ContextSpec.hs +++ b/test/unit/System/Taffybar/ContextSpec.hs @@ -24,10 +24,13 @@ ) where +import Control.Concurrent.MVar qualified as MV import Control.Exception (SomeException, bracket, catch) -import Control.Monad.Trans.Reader (runReaderT) +import Control.Monad.IO.Class (liftIO) +import Control.Monad.Trans.Reader (asks, runReaderT) import Data.Default (def) import Data.Ratio ((%)) +import Data.Unique (hashUnique, newUnique) import GHC.Generics (Generic) import GI.Gtk (Widget) import Network.Socket qualified as Socket @@ -240,6 +243,22 @@ lookupEnv "HYPRLAND_INSTANCE_SIGNATURE" `shouldReturn` Just liveSig removePathForcibly runtime `catch` (\(_ :: SomeException) -> pure ()) + describe "unsubscribe" $ do + it "removes only the listener with the given identifier" $ runTaffyNoX11 $ do + idA <- subscribeToAll (const $ return ()) + idB <- subscribeToAll (const $ return ()) + idC <- subscribeToAll (const $ return ()) + unsubscribe idB + remaining <- listenerIds + liftIO $ remaining `shouldMatchList` map hashUnique [idA, idC] + + it "leaves the listeners alone when the identifier is unknown" $ runTaffyNoX11 $ do + idA <- subscribeToAll (const $ return ()) + unknown <- liftIO newUnique + unsubscribe unknown + remaining <- listenerIds + liftIO $ remaining `shouldMatchList` [hashUnique idA] + describe "Fuzz tests" $ do prop "eval generators" prop_genSimpleConfig xprop "TaffybarConfig" prop_taffybarConfig @@ -260,6 +279,19 @@ runTaffyDefault :: TaffyIO a -> IO a runTaffyDefault f = buildContext def >>= runReaderT f + +-- | Run a 'TaffyIO' action in a context that has no X11 event loop attached. +-- The loop opens its own X11 connection and is never shut down, which makes +-- Xlib abort the whole test process once the test X server goes away. +runTaffyNoX11 :: TaffyIO a -> IO a +runTaffyNoX11 f = buildContextWithBackend BackendWayland def >>= runReaderT f + +-- | Hashes of the identifiers of the currently registered X11 event listeners. +-- 'Unique' has no 'Show' instance, so hspec list matchers need the hash. +listenerIds :: TaffyIO [Int] +listenerIds = do + listenersVar <- asks listeners + map (hashUnique . fst) <$> liftIO (MV.readMVar listenersVar) ------------------------------------------------------------------------