From a203b45a4b3f231fde0e579acb2fbd952539ff4e Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Thu, 1 Oct 2026 07:51:16 +0100 Subject: [PATCH] Restore portal appearance fallback to resolved disk configuration --- portal/src/settings.rs | 187 ++++++++++++++++++++----- scripts/tests/test_portal_contracts.py | 45 ++++-- 2 files changed, 185 insertions(+), 47 deletions(-) diff --git a/portal/src/settings.rs b/portal/src/settings.rs index 8a154f9..d450213 100644 --- a/portal/src/settings.rs +++ b/portal/src/settings.rs @@ -1,7 +1,10 @@ use std::collections::HashMap; +use std::io::Read; +use std::path::Path; use std::sync::Arc; use std::time::Duration; +use ferese_config::Document; use ferese_config::theme::{Appearance as ThemeAppearance, ResolvedTheme}; use tokio::sync::RwLock; use zbus::Connection; @@ -50,52 +53,60 @@ pub(crate) struct Settings(Arc>); impl Settings { pub(crate) fn new() -> Self { - Self(Arc::new(RwLock::new(Appearance::from_resolved( - &ferese_ipc::theme::current().theme, - )))) + let theme = ferese_ipc::theme::Connection::connect() + .ok() + .and_then(|mut connection| connection.get().ok()) + .map(|snapshot| snapshot.theme); + Self(Arc::new(RwLock::new(initial_appearance(theme.as_ref(), load_config)))) + } + + async fn update_appearance(&self, next: Appearance, emitter: &SignalEmitter<'_>) { + let mut current = self.0.write().await; + if *current == next { + return; + } + let previous = current.values(); + *current = next; + drop(current); + for (key, value) in next.values() { + if previous.get(&key) != Some(&value) { + let _ = Self::setting_changed(emitter, APPEARANCE, &key, value).await; + } + } } pub(crate) async fn watch(self, connection: Connection) { let emitter = SignalEmitter::new(&connection, PATH).unwrap(); loop { let connection = tokio::task::spawn_blocking(ferese_ipc::theme::Connection::connect).await; - let Ok(Ok(mut connection)) = connection else { - tokio::time::sleep(Duration::from_secs(1)).await; - continue; - }; - let Ok(cancellation) = connection.cancellation() else { - return; - }; - let (send, mut receive) = tokio::sync::mpsc::channel(1); - tokio::task::spawn_blocking(move || { - let Ok(mut snapshot) = connection.get() else { return }; - loop { - let revision = snapshot.revision; - if send.blocking_send(snapshot).is_err() { - return; - } - match connection.watch(revision) { - Ok(next) => snapshot = next, - Err(_) => return, - } - } - }); - while let Some(snapshot) = receive.recv().await { - let next = Appearance::from_resolved(&snapshot.theme); - let mut current = self.0.write().await; - if *current == next { - continue; - } - let previous = current.values(); - *current = next; - drop(current); - for (key, value) in next.values() { - if previous.get(&key) != Some(&value) { - let _ = Self::setting_changed(&emitter, APPEARANCE, &key, value).await; + if let Ok(Ok(mut connection)) = connection + && let Ok(cancellation) = connection.cancellation() + { + let (send, mut receive) = tokio::sync::mpsc::channel(1); + tokio::task::spawn_blocking(move || { + let Ok(mut snapshot) = connection.get() else { return }; + loop { + let revision = snapshot.revision; + if send.blocking_send(snapshot).is_err() { + return; + } + match connection.watch(revision) { + Ok(next) => snapshot = next, + Err(_) => return, + } } + }); + while let Some(snapshot) = receive.recv().await { + self.update_appearance(Appearance::from_resolved(&snapshot.theme), &emitter) + .await; } + drop(cancellation); + } + // Standalone startup and compositor disconnects use the same resolver. + // Rejected edits retain the last valid appearance, just as the owner does. + if let Ok(Some(next)) = tokio::task::spawn_blocking(load_config).await { + self.update_appearance(next, &emitter).await; } - drop(cancellation); tokio::time::sleep(Duration::from_secs(1)).await; } } @@ -147,10 +158,112 @@ fn matches_namespace(filters: &[String], namespace: &str) -> bool { }) } +fn initial_appearance(theme: Option<&ResolvedTheme>, load: impl FnOnce() -> Option) -> Appearance { + theme + .map(Appearance::from_resolved) + .or_else(load) + .unwrap_or_else(|| Appearance::from_resolved(&ResolvedTheme::default())) +} + +fn read_source(path: &Path) -> Result { + let mut source = String::new(); + std::fs::File::open(path) + .map_err(|error| error.to_string())? + .take(60 * 1024 + 1) + .read_to_string(&mut source) + .map_err(|error| error.to_string())?; + if source.len() > 60 * 1024 { + return Err(format!("{} exceeds 60 KiB", path.display())); + } + Ok(source) +} + +fn load_config() -> Option { + load_path(&ferese_config::config_path()?).ok() +} + +fn load_path(path: &Path) -> Result { + let source = match read_source(path) { + Ok(source) => source, + Err(_) if !path.try_exists().map_err(|error| error.to_string())? => { + return Ok(Appearance::from_resolved(&ResolvedTheme::default())); + } + Err(error) => return Err(error), + }; + let document = Document::parse(&source).map_err(|error| error.to_string())?; + let resolved = ferese_config::theme::resolve( + &document, + path.parent().unwrap_or_else(|| Path::new(".")), + jiff::Timestamp::now(), + read_source, + )?; + Ok(Appearance::from_resolved(&resolved.theme)) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn standalone_appearance_resolves_disk_config_and_imports() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("config.kdl"); + let imported = root.path().join("colors.kdl"); + std::fs::write(&imported, r##"colors { accent "#ff8000"; }"##).unwrap(); + let source = r#"theme { mode "light"; file "colors.kdl"; accessibility { increase-contrast #true; }; }; animations { reduced-motion #true; }"#; + std::fs::write(&path, source).unwrap(); + let appearance = initial_appearance(None, || load_path(&path).ok()); + let document = Document::parse(source).unwrap(); + let expected = + ferese_config::theme::resolve(&document, root.path(), jiff::Timestamp::now(), read_source).unwrap(); + assert_eq!(appearance, Appearance::from_resolved(&expected.theme)); + assert_eq!(appearance.scheme, 2); + assert_eq!(appearance.reduced_motion, 1); + assert_eq!(appearance.contrast, 1); + assert_ne!( + appearance.accent, + Appearance::from_resolved(&ResolvedTheme::default()).accent + ); + + std::fs::write(&path, r#"theme { mode "dark"; }; animations { enabled #false; }"#).unwrap(); + let next = load_path(&path).unwrap(); + assert_eq!(next.scheme, 1); + assert_eq!(next.reduced_motion, 1); + assert_eq!(next.contrast, 0); + } + + #[test] + fn ipc_appearance_has_priority_over_disk_fallback() { + let theme = ResolvedTheme { + appearance: ThemeAppearance::Light, + ..Default::default() + }; + assert_eq!( + initial_appearance(Some(&theme), || panic!("disk fallback read with live IPC")), + Appearance::from_resolved(&theme) + ); + } + + #[test] + fn disk_fallback_rejects_invalid_config_and_uses_defaults_when_absent() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("config.kdl"); + let default = Appearance::from_resolved(&ResolvedTheme::default()); + assert_eq!(load_path(&path).unwrap(), default); + for source in [ + r#"theme { broken"#, + r#"theme { mode "invalid"; }"#, + r#"theme { colors { accent "invalid"; }; }"#, + r#"theme { file "missing.kdl"; }"#, + ] { + std::fs::write(&path, source).unwrap(); + assert!(load_path(&path).is_err(), "{source}"); + assert_eq!(initial_appearance(None, || load_path(&path).ok()), default); + } + std::fs::write(&path, " ".repeat(60 * 1024 + 1)).unwrap(); + assert!(load_path(&path).is_err()); + } + #[test] fn namespaces_support_only_trailing_section_wildcards() { for filter in ["", "org.freedesktop.appearance", "org.freedesktop.*", "org.*"] { diff --git a/scripts/tests/test_portal_contracts.py b/scripts/tests/test_portal_contracts.py index 1b23165..2fbb377 100644 --- a/scripts/tests/test_portal_contracts.py +++ b/scripts/tests/test_portal_contracts.py @@ -20,12 +20,18 @@ PATH = "/org/freedesktop/portal/desktop" class NativePortalContracts(unittest.TestCase): def test_private_bus_contracts(self): - result = subprocess.run(["dbus-run-session", "--", sys.executable, __file__, "--private"], + self.run_private("--private") + + def test_standalone_disk_appearance_contracts(self): + self.run_private("--private-standalone") + + def run_private(self, mode): + result = subprocess.run(["dbus-run-session", "--", sys.executable, __file__, mode], capture_output=True, text=True, timeout=45) self.assertEqual(result.returncode, 0, result.stdout + result.stderr) -def private_bus_checks(): +def private_bus_checks(standalone=False): import gi gi.require_version("Gio", "2.0") from gi.repository import Gio, GLib @@ -41,11 +47,14 @@ def private_bus_checks(): root = Path(directory) config = root / "ferese/config.kdl" config.parent.mkdir() - # Lockdown still uses this document; appearance comes exclusively from theme IPC. - config.write_text('theme { mode "dark"; }; animations { reduced-motion #false; }\n') + if standalone: + config.write_text('theme { mode "light"; colors { surface-base "#ffffff"; accent "#ff8000"; }; }; animations { reduced-motion #true; }\n') + else: + # A live owner takes priority over this deliberately different document. + config.write_text('theme { mode "dark"; }; animations { reduced-motion #false; }\n') runtime = root / "runtime" runtime.mkdir(mode=0o700) - theme_owner = ThemeOwner(runtime) + theme_owner = None if standalone else ThemeOwner(runtime) log = (root / "backend.log").open("w") env = dict(os.environ, XDG_CONFIG_HOME=directory, XDG_RUNTIME_DIR=str(runtime), WAYLAND_DISPLAY="wayland-unavailable") @@ -83,7 +92,12 @@ def private_bus_checks(): values, = call("org.freedesktop.impl.portal.Settings", "ReadAll", GLib.Variant("(as)", ([],)), "(a{sa{sv}})") appearance = values["org.freedesktop.appearance"] assert appearance["color-scheme"] == 2 and appearance["reduced-motion"] == 1, appearance - assert appearance["accent-color"][0] == 1. and appearance["accent-color"][2] == 0., appearance + if standalone: + # The resolver adjusts the requested orange for contrast in Light mode. + red, green, blue = appearance["accent-color"] + assert 1 >= red > green > blue >= 0, appearance + else: + assert appearance["accent-color"][0] == 1. and appearance["accent-color"][2] == 0., appearance result, = call("org.freedesktop.impl.portal.Settings", "ReadAll", GLib.Variant("(as)", (["other.*"],)), "(a{sa{sv}})") assert result == {} try: @@ -229,18 +243,26 @@ def private_bus_checks(): signals = [] subscription = bus.signal_subscribe(NAME, "org.freedesktop.impl.portal.Settings", "SettingChanged", PATH, None, Gio.DBusSignalFlags.NONE, lambda *args: signals.append(args[-1].unpack())) - theme_owner.publish("dark", "#0080ff", False, True) + if standalone: + temporary = config.with_suffix(".tmp") + temporary.write_text('theme { mode "dark"; colors { accent "#0080ff"; }; accessibility { increase-contrast #true; }; }; animations { enabled #true; }\n') + temporary.replace(config) + else: + theme_owner.publish("dark", "#0080ff", False, True) deadline = time.monotonic() + 5 while time.monotonic() < deadline and not {"color-scheme", "accent-color", "reduced-motion", "contrast"}.issubset({signal[1] for signal in signals}): GLib.MainContext.default().iteration(False) time.sleep(0.02) assert any(signal[1] == "color-scheme" and signal[2] == 1 for signal in signals), signals changed = {event[1]: event[2] for event in signals} - assert changed["accent-color"] == (0., 128 / 255, 1.), changed + if standalone: + assert changed["accent-color"] != appearance["accent-color"], changed + else: + assert changed["accent-color"] == (0., 128 / 255, 1.), changed assert changed["reduced-motion"] == 0 and changed["contrast"] == 1, changed current, = call("org.freedesktop.impl.portal.Settings", "ReadAll", GLib.Variant("(as)", (["org.freedesktop.*"],)), "(a{sa{sv}})") assert current["org.freedesktop.appearance"] == changed, current - # Portal appearance stays with the owner even when the local document is invalid. + # An invalid edit retains the last valid appearance for either source. config.write_text('theme { broken') time.sleep(1.2) scheme, = call("org.freedesktop.impl.portal.Settings", "Read", GLib.Variant("(ss)", ("org.freedesktop.appearance", "color-scheme"))) @@ -250,11 +272,14 @@ def private_bus_checks(): os.killpg(process.pid, signal.SIGTERM) process.wait(timeout=5) log.close() - theme_owner.close() + if theme_owner is not None: + theme_owner.close() if __name__ == "__main__": if sys.argv[1:] == ["--private"]: private_bus_checks() + elif sys.argv[1:] == ["--private-standalone"]: + private_bus_checks(standalone=True) else: unittest.main()