Keep the theme on the account, not in the browser

- users.theme and users.theme_mode, added by migrate(); GET /api/me returns them
  and PATCH /api/me saves them, refusing anything but a plain name and
  light/dark/auto, since index() writes them into the page's <html> tag.
- The page arrives with data-theme and data-choice already on <html> (and
  data-mode unless Auto), so it is drawn in the account's theme from the start.
- A theme a browser kept in localStorage goes up to the account once, the first
  time an account with none loads the page.
- Saves go one at a time, each with the choice as it stands: sent all at once, a
  quick run through the list could land out of order and keep a theme passed on
  the way. The browser test caught it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-18 14:33:59 +00:00
parent 9b2537761f
commit 9c16408d04
6 changed files with 164 additions and 27 deletions

View File

@@ -16,6 +16,9 @@ The long form, with what was wrong before and how it was found, is in
Classic and Modern (the existing dark and light). Each that comes both ways has its own Light, Classic and Modern (the existing dark and light). Each that comes both ways has its own Light,
Dark or Auto setting; Classic and Paper come one way only, so that setting is hidden for them. Dark or Auto setting; Classic and Paper come one way only, so that setting is hidden for them.
A theme chosen before this carries over. A theme chosen before this carries over.
- Your theme is kept on your account rather than in the browser, so it follows you to another
browser or computer, and the page arrives in it with no flash of the default. The theme a
browser already had is saved to your account the first time you load the page.
- Touch gestures: pull the item list down from its top to check the feed for new items, and - Touch gestures: pull the item list down from its top to check the feed for new items, and
swipe the item you are reading left for the next one and right for the one before, or back swipe the item you are reading left for the next one and right for the one before, or back
to the list from the first. to the list from the first.

View File

@@ -81,7 +81,10 @@ CREATE TABLE IF NOT EXISTS users (
is_admin INTEGER NOT NULL DEFAULT 0, is_admin INTEGER NOT NULL DEFAULT 0,
-- For whoever maintains the server. NULL where it is not known. -- For whoever maintains the server. NULL where it is not known.
created INTEGER, created INTEGER,
last_login INTEGER last_login INTEGER,
-- The theme chosen in Settings, and light, dark or auto. NULL until one is chosen.
theme TEXT,
theme_mode TEXT
); );
-- What one person wants from a feed. The feed, its items and its files are shared; this -- What one person wants from a feed. The feed, its items and its files are shared; this
@@ -176,6 +179,9 @@ fn migrate(conn: &Connection) -> Result<()> {
("feeds", "error_since", "INTEGER"), ("feeds", "error_since", "INTEGER"),
("feeds", "category", "TEXT"), ("feeds", "category", "TEXT"),
("entry_state", "duration", "INTEGER"), ("entry_state", "duration", "INTEGER"),
// Kept per account so a theme follows you to another browser; it was in localStorage.
("users", "theme", "TEXT"),
("users", "theme_mode", "TEXT"),
]; ];
let retired: &[(&str, &str)] = &[ let retired: &[(&str, &str)] = &[
// Read state from before accounts, long since moved to entry_state. Two bugs came from // Read state from before accounts, long since moved to entry_state. Two bugs came from
@@ -1180,6 +1186,23 @@ impl Db {
Ok(()) Ok(())
} }
/// The theme this person chose, and light, dark or auto; None for either until they choose.
pub fn theme(&self, user_id: i64) -> Result<(Option<String>, Option<String>)> {
let conn = self.conn.lock().unwrap();
Ok(conn.query_row("SELECT theme, theme_mode FROM users WHERE id = ?1", [user_id], |r| {
Ok((r.get(0)?, r.get(1)?))
})?)
}
pub fn set_theme(&self, user_id: i64, theme: &str, mode: &str) -> Result<()> {
let conn = self.conn.lock().unwrap();
conn.execute(
"UPDATE users SET theme = ?2, theme_mode = ?3 WHERE id = ?1",
params![user_id, theme, mode],
)?;
Ok(())
}
/// The user behind a session cookie, if it is still live. Idle sessions expire after /// The user behind a session cookie, if it is still live. Idle sessions expire after
/// `max_idle_secs`; touching `seen` is what keeps a session in daily use alive. /// `max_idle_secs`; touching `seen` is what keeps a session in daily use alive.
pub fn session_user(&self, token: &str, max_idle_secs: i64) -> Result<Option<User>> { pub fn session_user(&self, token: &str, max_idle_secs: i64) -> Result<Option<User>> {

View File

@@ -36,7 +36,7 @@ pub struct WebState {
pub fn router(state: WebState) -> Router { pub fn router(state: WebState) -> Router {
Router::new() Router::new()
.route("/", get(index)) .route("/", get(index))
.route("/api/me", get(me)) .route("/api/me", get(me).patch(patch_me))
.route("/api/logout", post(logout)) .route("/api/logout", post(logout))
.route("/api/feeds", get(feeds).post(add_feed)) .route("/api/feeds", get(feeds).post(add_feed))
.route("/api/feeds/{id}", patch(patch_feed).delete(remove_feed)) .route("/api/feeds/{id}", patch(patch_feed).delete(remove_feed))
@@ -317,7 +317,37 @@ async fn me(
) -> Json<serde_json::Value> { ) -> Json<serde_json::Value> {
let url = state.ctx.cfg().web.sign_out_url.clone(); let url = state.ctx.cfg().web.sign_out_url.clone();
let sign_out = (by_proxy && !url.is_empty()).then_some(url); let sign_out = (by_proxy && !url.is_empty()).then_some(url);
Json(serde_json::json!({ "name": user.name, "admin": user.is_admin, "sign_out": sign_out })) let (theme, mode) = state.ctx.db.theme(user.id).unwrap_or_default();
Json(serde_json::json!({
"name": user.name, "admin": user.is_admin, "sign_out": sign_out, "theme": theme, "mode": mode,
}))
}
#[derive(Deserialize)]
struct MePatch {
theme: String,
mode: String,
}
/// Saves the theme to the account, so it follows the person rather than the browser.
async fn patch_me(
State(state): State<WebState>,
user: crate::db::User,
Json(body): Json<MePatch>,
) -> Result<StatusCode, ApiError> {
// The page's script knows the themes; this only makes sure what is kept is safe to write
// into the page's <html> tag, which is where index() puts it.
if !theme_ok(&body.theme, &body.mode) {
return Err(ApiError::bad_request("not a theme"));
}
state.ctx.db.set_theme(user.id, &body.theme, &body.mode)?;
Ok(StatusCode::NO_CONTENT)
}
fn theme_ok(theme: &str, mode: &str) -> bool {
(1..=20).contains(&theme.len())
&& theme.bytes().all(|b| b.is_ascii_lowercase())
&& ["light", "dark", "auto"].contains(&mode)
} }
// ---- accounts: admin only ---- // ---- accounts: admin only ----
@@ -524,16 +554,29 @@ const LOG_BUTTON: &str = "<button id=logs ";
/// The page, with the log button left out for anyone but an admin. Hiding it from the page's /// The page, with the log button left out for anyone but an admin. Hiding it from the page's
/// script instead showed it for a moment on every load, until /api/me answered. /// script instead showed it for a moment on every load, until /api/me answered.
async fn index(user: crate::db::User) -> impl IntoResponse { async fn index(State(state): State<WebState>, user: crate::db::User) -> impl IntoResponse {
([(header::CACHE_CONTROL, PAGE_CACHE)], Html(page_for(user.is_admin))) let theme = state.ctx.db.theme(user.id).unwrap_or_default();
([(header::CACHE_CONTROL, PAGE_CACHE)], Html(page_for(user.is_admin, theme)))
} }
fn page_for(admin: bool) -> std::borrow::Cow<'static, str> { const HTML_TAG: &str = "<html lang=en>";
if admin {
INDEX.into() /// The page for this person: their theme on its <html> tag, so the page is drawn in it from the
} else { /// first frame on any browser, and without the log button unless they are an admin.
INDEX.replacen(LOG_BUTTON, "<button id=logs hidden ", 1).into() fn page_for(admin: bool, theme: (Option<String>, Option<String>)) -> String {
let mut page = INDEX.to_owned();
if let (Some(t), Some(m)) = theme
&& theme_ok(&t, &m)
{
// data-choice is light, dark or auto; data-mode, what the CSS reads, is only known here
// for the first two. theme.ts works Auto out from the system.
let mode = if m == "auto" { String::new() } else { format!(" data-mode={m}") };
page = page.replacen(HTML_TAG, &format!("<html lang=en data-theme={t} data-choice={m}{mode}>"), 1);
} }
if !admin {
page = page.replacen(LOG_BUTTON, "<button id=logs hidden ", 1);
}
page
} }
#[derive(Serialize)] #[derive(Serialize)]
@@ -871,8 +914,19 @@ mod tests {
#[test] #[test]
fn only_an_admin_is_sent_the_log_button() { fn only_an_admin_is_sent_the_log_button() {
// If the markup drifts from LOG_BUTTON, replacen matches nothing and says nothing. // If the markup drifts from LOG_BUTTON, replacen matches nothing and says nothing.
assert!(page_for(false).contains("<button id=logs hidden ")); assert!(page_for(false, (None, None)).contains("<button id=logs hidden "));
assert!(!page_for(true).contains("id=logs hidden")); assert!(!page_for(true, (None, None)).contains("id=logs hidden"));
}
#[test]
fn the_page_arrives_in_the_theme_the_account_chose() {
let page = |t: &str, m: &str| page_for(true, (Some(t.into()), Some(m.into())));
// If the minifier ever writes the tag differently, HTML_TAG matches nothing, silently.
assert!(page("dracula", "dark").contains("<html lang=en data-theme=dracula data-choice=dark data-mode=dark>"));
assert!(page("nordic", "auto").contains("<html lang=en data-theme=nordic data-choice=auto>"));
// Whatever is in the column is written into markup, so only a plain name gets there.
assert!(!page("x onload=alert(1)", "dark").contains("onload"));
assert!(page("modern", "dark\"").contains("<html lang=en>"));
} }
#[test] #[test]

View File

@@ -63,8 +63,12 @@ test('Settings picks a theme and, where it has both, light, dark or Auto', async
await page.locator('#stheme').selectOption('paper'); await page.locator('#stheme').selectOption('paper');
await expect(page.locator('#smode')).toBeHidden(); await expect(page.locator('#smode')).toBeHidden();
await expect.poll(bg).toBe('rgb(242, 238, 222)'); // #F2EEDE await expect.poll(bg).toBe('rgb(242, 238, 222)'); // #F2EEDE
// The save that says Classic: the ones before it may still be answering.
const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH'
&& r.request().postDataJSON().theme === 'classic');
await page.locator('#stheme').selectOption('classic'); await page.locator('#stheme').selectOption('classic');
await expect(page.locator('#smode')).toBeHidden(); await expect(page.locator('#smode')).toBeHidden();
await saved; // kept on the account, not the browser
await page.reload(); await page.reload();
await expect.poll(root).toEqual(['classic', 'light']); await expect.poll(root).toEqual(['classic', 'light']);
// The 2004 Mac app set its type in Lucida Grande. // The 2004 Mac app set its type in Lucida Grande.
@@ -79,11 +83,46 @@ test('Settings picks a theme and, where it has both, light, dark or Auto', async
await expect.poll(bg).toBe('rgb(46, 52, 64)'); // nord0 await expect.poll(bg).toBe('rgb(46, 52, 64)'); // nord0
}); });
test('a theme saved before there was light and dark carries over', async ({ page }) => { test('the theme is kept on the account, and follows it to another browser', async ({ page, browser }) => {
await page.evaluate(() => { localStorage.setItem('ipx.theme', 'light'); localStorage.removeItem('ipx.mode'); }); await page.locator('#prefs').click();
await page.reload(); const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH');
await page.locator('#stheme').selectOption('flatremix');
expect((await saved).status()).toBe(204);
const saved2 = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH');
await page.locator('#smode').selectOption('light');
await saved2;
// Another browser: nothing in its localStorage, and the page still arrives in the theme,
// written onto <html> by the server rather than set once the script has run.
const other = await browser.newContext();
const p2 = await other.newPage();
const res = await p2.goto(`/?token=${TOKEN}`);
expect(await res.text()).toContain('data-theme=flatremix data-choice=light data-mode=light');
expect(await p2.evaluate(() => [document.documentElement.dataset.theme, document.documentElement.dataset.mode]))
.toEqual(['flatremix', 'light']);
await other.close();
});
test('a theme this browser kept before themes were on the account goes up to it once', async ({ browser }) => {
// A new account, made by the proxy header on first sight, so it has no theme of its own yet.
const who = `theme-${Date.now()}@example.com`;
const ctx = await browser.newContext({ extraHTTPHeaders: { 'X-Test-User': who } });
// From before light and dark: ipx.theme alone, 'light' meaning Modern, light.
await ctx.addInitScript(() => { localStorage.setItem('ipx.theme', 'light'); localStorage.removeItem('ipx.mode'); });
const page = await ctx.newPage();
const saved = page.waitForResponse(r => r.url().endsWith('/api/me') && r.request().method() === 'PATCH');
await page.goto('/');
expect(await page.evaluate(() => [document.documentElement.dataset.theme, document.documentElement.dataset.mode])) expect(await page.evaluate(() => [document.documentElement.dataset.theme, document.documentElement.dataset.mode]))
.toEqual(['modern', 'light']); .toEqual(['modern', 'light']);
expect((await saved).request().postDataJSON()).toEqual({ theme: 'modern', mode: 'light' });
await ctx.close();
// Anywhere else now, it comes from the account.
const fresh = await browser.newContext({ extraHTTPHeaders: { 'X-Test-User': who } });
const p2 = await fresh.newPage();
const res = await p2.goto('/');
expect(await res.text()).toContain('data-theme=modern data-choice=light');
await fresh.close();
}); });
test('settings opens and saves the global schedule', async ({ page }) => { test('settings opens and saves the global schedule', async ({ page }) => {

View File

@@ -379,8 +379,8 @@ async function prefsModal(){
<span class="hint">Add and remove the people who can sign in, and choose who is an admin.</span></div>`:''} <span class="hint">Add and remove the people who can sign in, and choose who is an admin.</span></div>`:''}
<div class="cardacts"><button class="btn ico" onclick="closeModal()" title="${admin?'Cancel':'Close'}" aria-label="${admin?'Cancel':'Close'}">${ICON.close}</button> <div class="cardacts"><button class="btn ico" onclick="closeModal()" title="${admin?'Cancel':'Close'}" aria-label="${admin?'Cancel':'Close'}">${ICON.close}</button>
${admin?`<button class="btn ico primary" id="gsave" title="Save" aria-label="Save">${ICON.check}</button>`:''}</div>`); ${admin?`<button class="btn ico primary" id="gsave" title="Save" aria-label="Save">${ICON.check}</button>`:''}</div>`);
$('#stheme').onchange=e=>setTheme(e.target.value); $('#stheme').onchange=e=>setTheme(e.target.value,undefined,true);
$('#smode').onchange=e=>setTheme(undefined,e.target.value); $('#smode').onchange=e=>setTheme(undefined,e.target.value,true);
$('#gopml').onclick=opmlModal; $('#gopml').onclick=opmlModal;
if(!admin) return; if(!admin) return;
$('#gusers').onclick=usersModal; $('#gusers').onclick=usersModal;

View File

@@ -1,7 +1,9 @@
/* ---------------- theme ---------------- */ /* ---------------- theme ---------------- */
// A theme, and for those that come in both, light, dark or Auto, chosen in Settings and kept in // A theme, and for those that come in both, light, dark or Auto, chosen in Settings and kept on
// ipx.theme and ipx.mode. The page gets data-theme and data-mode, light or dark, which is all // the account, so it follows you to another browser or computer. The server writes it onto the
// the CSS reads: Auto is worked out here, from the system, so no palette is written twice. // page's <html> tag (data-theme, data-choice) so the page is drawn in it from the start. The
// page gets data-mode, light or dark, which is all the CSS reads: Auto is worked out here, from
// the system, so no palette is written twice.
const THEMES: Record<string, {name: string, modes: boolean}> = { const THEMES: Record<string, {name: string, modes: boolean}> = {
modern: {name: 'Modern', modes: true}, modern: {name: 'Modern', modes: true},
classic: {name: 'Classic, the 2004 Mac app', modes: false}, classic: {name: 'Classic, the 2004 Mac app', modes: false},
@@ -13,12 +15,13 @@ const THEMES: Record<string, {name: string, modes: boolean}> = {
nordic: {name: 'Nordic', modes: true}, nordic: {name: 'Nordic', modes: true},
}; };
const MODES: Record<string, string> = {auto: 'Auto (matches your system)', light: 'Light', dark: 'Dark'}; const MODES: Record<string, string> = {auto: 'Auto (matches your system)', light: 'Light', dark: 'Dark'};
// Before themes came in light and dark, ipx.theme held one of these. // Before themes came in light and dark, ipx.theme in localStorage held one of these.
const OLD_THEMES: Record<string, [string, string]> = {dark: ['modern', 'dark'], light: ['modern', 'light'], auto: ['modern', 'auto']}; const OLD_THEMES: Record<string, [string, string]> = {dark: ['modern', 'dark'], light: ['modern', 'light'], auto: ['modern', 'auto']};
const systemDark = window.matchMedia?.('(prefers-color-scheme: dark)'); const systemDark = window.matchMedia?.('(prefers-color-scheme: dark)');
const theme = {name: 'modern', mode: 'dark'}; const theme = {name: 'modern', mode: 'dark'};
function setTheme(name = theme.name, mode = theme.mode){ /// `save` for a choice made in Settings, which goes to the account; not for applying one.
function setTheme(name = theme.name, mode = theme.mode, save = false){
theme.name = THEMES[name] ? name : 'modern'; theme.name = THEMES[name] ? name : 'modern';
theme.mode = MODES[mode] ? mode : 'dark'; theme.mode = MODES[mode] ? mode : 'dark';
const both = THEMES[theme.name].modes; const both = THEMES[theme.name].modes;
@@ -30,11 +33,26 @@ function setTheme(name = theme.name, mode = theme.mode){
const sel = $('#stheme'); if(sel) sel.value = theme.name; const sel = $('#stheme'); if(sel) sel.value = theme.name;
const ms = $('#smode'); if(ms) ms.value = theme.mode; const ms = $('#smode'); if(ms) ms.value = theme.mode;
const mf = $('#smodefield'); if(mf) mf.hidden = !both; const mf = $('#smodefield'); if(mf) mf.hidden = !both;
try{ localStorage.setItem('ipx.theme', theme.name); localStorage.setItem('ipx.mode', theme.mode); }catch{} if(save) saveTheme();
}
/// One save at a time, each sending the choice as it stands when it goes. Sent as they came,
/// several at once, a quick run through the list could reach the server out of order and
/// leave the account on a theme passed on the way.
let themeSaving = Promise.resolve();
function saveTheme(){
themeSaving = themeSaving
.then(() => api('/api/me', {method: 'PATCH', body: JSON.stringify({theme: theme.name, mode: theme.mode})}))
.catch(e => toast(`Your theme was not saved: ${e.message}`, true));
} }
systemDark?.addEventListener?.('change', () => { if(theme.mode === 'auto') setTheme(); }); systemDark?.addEventListener?.('change', () => { if(theme.mode === 'auto') setTheme(); });
try{ (() => {
let name = localStorage.getItem('ipx.theme'), mode = localStorage.getItem('ipx.mode'); const root = document.documentElement;
if(root.dataset.choice) return setTheme(root.dataset.theme, root.dataset.choice);
// Nothing on the account yet. A theme this browser kept, from before themes were kept on the
// account, goes up to it once, so nobody has to choose again.
let name: string | null = null, mode: string | null = null;
try{ name = localStorage.getItem('ipx.theme'); mode = localStorage.getItem('ipx.mode'); }catch{}
if(OLD_THEMES[name]) [name, mode] = OLD_THEMES[name]; if(OLD_THEMES[name]) [name, mode] = OLD_THEMES[name];
setTheme(name ?? undefined, mode ?? undefined); setTheme(name ?? undefined, mode ?? undefined, !!name);
}catch{ setTheme(); } })();