Fix the open bugs: read state, Unread tab, feed errors, theme button, log button, relative images

- Opening an item stays read: a list refresh that crossed with the write no longer puts the unread dot back (#16). - On the Unread tab the item you were reading goes when you move to the next (#17). - Feed errors mark the feed with a red ! instead of a toast per failure (#20). - The theme is chosen in Settings only (#15). - The server leaves the Log button out of a non-admin's page, so it no longer flashes (#29). - Relative images and links in a post resolve against the post's link (#28). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-18 12:10:05 +00:00
parent 0443177471
commit d7a4a0b663
4 changed files with 133 additions and 53 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,23 @@ The long form, with what was wrong before and how it was found, is in

 ## [Unreleased]

+### Changed
+
+- A feed that fails to check gets a red ! in the feed list, and its page says why, in place of a
+  pop-up per failure that everyone saw during a scan of every feed.
+- The theme is chosen in Settings only; the button beside the iPodderX name is gone.
+
+### Fixed
+
+- An item you open stays read. A list refresh that crossed with marking it read could put its
+  unread dot back until the next refresh.
+- On the Unread tab, the item you were reading leaves the list as soon as you move to the next
+  one, rather than a few read items lingering until a refresh cleared them.
+- The Log button no longer shows for a moment on every load for anyone but an admin; the server
+  leaves it out of their page.
+- An image or link in a post given relative to the post, such as The Observation Deck's, now
+  points at the post's site rather than at ipx, and shows.
+
 ## [0.6.1] - 2026-09-15

 ### Fixed
--- a/src/web.rs
+++ b/src/web.rs
@@ -471,8 +471,21 @@ fn constant_time_eq(a: &str, b: &str) -> bool {
    a.iter().zip(b).fold(0u8, |acc, (x, y)| acc | (x ^ y)) == 0
 }

-async fn index() -> Html<&'static str> {
-    Html(include_str!("../web/index.html"))
+const INDEX: &str = include_str!("../web/index.html");
+const LOG_BUTTON: &str = r#"<button id="logs" "#;
+
+/// 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.
+async fn index(user: crate::db::User) -> Html<std::borrow::Cow<'static, str>> {
+    Html(page_for(user.is_admin))
+}
+
+fn page_for(admin: bool) -> std::borrow::Cow<'static, str> {
+    if admin {
+        INDEX.into()
+    } else {
+        INDEX.replacen(LOG_BUTTON, r#"<button id="logs" hidden "#, 1).into()
+    }
 }

 #[derive(Serialize)]
@@ -795,6 +808,24 @@ impl IntoResponse for ApiError {
 mod tests {
    use super::*;

+    #[test]
+    fn a_relative_image_resolves_against_the_post() {
+        let mut b = ammonia::Builder::new();
+        let html = r#"<img src="images/a.webp"><a href="/about">x</a><script>bad()</script>"#;
+        let out = clean_description(&mut b, html, Some("https://example.com/2026/04/post/"));
+        assert!(out.contains(r#"src="https://example.com/2026/04/post/images/a.webp""#), "{out}");
+        assert!(out.contains(r#"href="https://example.com/about""#), "{out}");
+        assert!(!out.contains("bad()"), "{out}");
+        assert!(clean_description(&mut b, html, None).contains(r#"src="images/a.webp""#));
+    }
+
+    #[test]
+    fn only_an_admin_is_sent_the_log_button() {
+        // If the markup drifts from LOG_BUTTON, replacen matches nothing and says nothing.
+        assert!(page_for(false).contains(r#"<button id="logs" hidden "#));
+        assert!(!page_for(true).contains(r#"id="logs" hidden"#));
+    }
+
    #[test]
    fn a_feed_with_a_credential_is_never_popular() {
        let f = |url: &str| crate::config::Feed {
@@ -974,13 +1005,24 @@ fn entry_page(
    sanitizer.add_tag_attributes("a", &["target"]).set_tag_attribute_value("a", "target", "_blank");
    for row in &mut rows {
        if let Some(d) = &row.description {
-            row.description = Some(sanitizer.clean(d).to_string());
+            row.description = Some(clean_description(&mut sanitizer, d, row.link.as_deref()));
        }
    }
    let total = db.count_in(user_id, feed, filter, search)?;
    Ok(Json(EntryPage { total, entries: rows }))
 }

+/// A relative `src` or `href` in a post means relative to the post, not to ipx: The
+/// Observation Deck's `images/37k-a-day-bro.webp` came up as a broken image.
+fn clean_description(sanitizer: &mut ammonia::Builder, html: &str, link: Option<&str>) -> String {
+    let base = link.and_then(|l| url::Url::parse(l).ok());
+    sanitizer.url_relative(match base {
+        Some(b) => ammonia::UrlRelative::RewriteWithBase(b),
+        None => ammonia::UrlRelative::PassThrough,
+    });
+    sanitizer.clean(html).to_string()
+}
+
 #[derive(Deserialize)]
 struct NewFeed {
    url: String,
--- a/tests/ui/app.spec.js
+++ b/tests/ui/app.spec.js
@@ -20,27 +20,6 @@ test('the page loads and lists the configured feeds', async ({ page }) => {
  expect(errors, 'the page script must not throw at load').toEqual([]);
 });

-test('the theme toggle actually changes the theme', async ({ page }) => {
-  // Regression: this button was wired after a line that threw, so it did nothing.
-  const before = await page.evaluate(() => document.documentElement.dataset.theme || 'system');
-  await page.locator('#theme').click();
-  await expect
-    .poll(() => page.evaluate(() => document.documentElement.dataset.theme))
-    .not.toBe(before);
-});
-
-test('the theme button steps through dark, light and classic, and remembers', async ({ page }) => {
-  const theme = () => page.evaluate(() => document.documentElement.dataset.theme);
-  for (let i = 0; i < 3 && (await theme()) !== 'classic'; i++) await page.locator('#theme').click();
-  expect(await theme()).toBe('classic');
-  await expect(page.locator('#theme')).toHaveAttribute('title', /Classic.*Click for Auto/);
-
-  await page.reload();
-  await expect.poll(theme).toBe('classic');
-  // The 2004 Mac app set its type in Lucida Grande.
-  expect(await page.evaluate(() => getComputedStyle(document.body).fontFamily)).toContain('Lucida Grande');
-});
-
 test('the theme dropdown in Settings jumps straight to a theme, including Auto', async ({ page }) => {
  const theme = () => page.evaluate(() => document.documentElement.dataset.theme);
  await page.locator('#prefs').click();
@@ -57,10 +36,12 @@ test('the theme dropdown in Settings jumps straight to a theme, including Auto',
  await expect.poll(() => page.evaluate(() => getComputedStyle(document.body).backgroundColor))
    .toBe('rgb(14, 19, 27)');      // the bare :root is already dark; Auto adds nothing here

-  // The header button and the dropdown are the same one setting, not two.
-  await page.locator('#modalCard .cardacts .btn').first().click();   // Cancel, closing the modal
-  await page.locator('#theme').click();
-  expect(await theme()).toBe('dark');
+  await page.locator('#stheme').selectOption('classic');
+  await page.reload();
+  await expect.poll(theme).toBe('classic');
+  // The 2004 Mac app set its type in Lucida Grande.
+  expect(await page.evaluate(() => getComputedStyle(document.body).fontFamily)).toContain('Lucida Grande');
+  expect(await page.locator('#theme').count(), 'the theme lives in Settings only').toBe(0);
 });

 test('settings opens and saves the global schedule', async ({ page }) => {
@@ -233,6 +214,32 @@ test('the filter tabs change what is listed', async ({ page }) => {
  await expect(page.locator('#count')).toContainText('0 items');
 });

+test('on the Unread tab an item stays while you read it and goes when you move on', async ({ page }) => {
+  await page.locator('.feed', { hasText: 'Test Show' }).click();
+  await page.locator('.tabs button', { hasText: 'All' }).first().click();
+  await expect(page.locator('.ep').nth(1)).toBeVisible({ timeout: 20_000 });
+  // Earlier tests read things; make the first two unread with their own dots.
+  for (const i of [0, 1]) {
+    const row = page.locator('.ep').nth(i);
+    if (await row.evaluate(r => r.classList.contains('read'))) {
+      await row.locator('[data-a="read"]').click();
+      await expect(page.locator('.ep').nth(i)).not.toHaveClass(/\bread\b/);
+    }
+  }
+  await page.locator('.tabs button', { hasText: 'Unread' }).first().click();
+  const first = page.locator('.ep').first();
+  const guid = await first.getAttribute('data-guid');
+  await first.click();
+  const it = page.locator(`.ep[data-guid="${guid}"]`);
+  await expect(it).toHaveClass(/\bread\b/);
+  await page.waitForTimeout(1500);   // past the SSE refresh debounce and loadFeeds
+  await expect(it).toBeVisible();
+
+  await page.locator('.ep').nth(1).click();
+  await expect(it).toHaveCount(0);
+  await page.locator('.tabs button', { hasText: 'All' }).first().click();
+});
+
 test('a feed URL is editable and has a copy button', async ({ page }) => {
  await page.locator('.feed', { hasText: 'Test Show' }).click();
  await page.locator('#content .acts [data-a="settings"]').click();
--- a/web/index.html
+++ b/web/index.html
@@ -670,7 +670,6 @@ input[type=range]::-moz-range-thumb{width:12px;height:12px;border:0;border-radiu
  <aside id="sidebar">
    <div class="brand">
      <img class="logo" src="/icon.png" alt="iPodderX" title="The original iPodderX icon, 2004"><h1>iPodderX</h1>
-      <button class="iconbtn" id="theme" title="Theme" aria-label="Theme" data-icon="theme"></button>
    </div>
    <div class="searchwrap"><input type="search" id="feedFilter" placeholder="Filter feeds…"></div>
    <div id="feedlist"></div>
@@ -750,7 +749,6 @@ const ICON={
  log:fa('0 0 384 512','<path fill="currentColor" d="M0 64C0 28.7 28.7 0 64 0L213.5 0c17 0 33.3 6.7 45.3 18.7L365.3 125.3c12 12 18.7 28.3 18.7 45.3L384 448c0 35.3-28.7 64-64 64L64 512c-35.3 0-64-28.7-64-64L0 64zm208-5.5l0 93.5c0 13.3 10.7 24 24 24L325.5 176 208 58.5zM120 256c-13.3 0-24 10.7-24 24s10.7 24 24 24l144 0c13.3 0 24-10.7 24-24s-10.7-24-24-24l-144 0zm0 96c-13.3 0-24 10.7-24 24s10.7 24 24 24l144 0c13.3 0 24-10.7 24-24s-10.7-24-24-24l-144 0z"/>'), // solid/file-lines
  close:fa('0 0 384 512','<path fill="currentColor" d="M55.1 73.4c-12.5-12.5-32.8-12.5-45.3 0s-12.5 32.8 0 45.3L147.2 256 9.9 393.4c-12.5 12.5-12.5 32.8 0 45.3s32.8 12.5 45.3 0L192.5 301.3 329.9 438.6c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L237.8 256 375.1 118.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0L192.5 210.7 55.1 73.4z"/>'), // solid/xmark
  menu:fa('0 0 448 512','<path fill="currentColor" d="M0 96C0 78.3 14.3 64 32 64l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L32 128C14.3 128 0 113.7 0 96zM0 256c0-17.7 14.3-32 32-32l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L32 288c-17.7 0-32-14.3-32-32zM448 416c0 17.7-14.3 32-32 32L32 448c-17.7 0-32-14.3-32-32s14.3-32 32-32l384 0c17.7 0 32 14.3 32 32z"/>'), // solid/bars
-  theme:fa('0 0 512 512','<path fill="currentColor" d="M448 256c0-106-86-192-192-192l0 384c106 0 192-86 192-192zM0 256a256 256 0 1 1 512 0 256 256 0 1 1 -512 0z"/>'), // solid/circle-half-stroke
  directory:fa('0 0 448 512','<path fill="currentColor" d="M0 96C0 60.7 28.7 32 64 32l320 0c35.3 0 64 28.7 64 64l0 320c0 35.3-28.7 64-64 64L64 480c-35.3 0-64-28.7-64-64L0 96zm64 0l0 64 64 0 0-64-64 0zm320 0l-192 0 0 64 192 0 0-64zM64 224l0 64 64 0 0-64-64 0zm320 0l-192 0 0 64 192 0 0-64zM64 352l0 64 64 0 0-64-64 0zm320 0l-192 0 0 64 192 0 0-64z"/>'), // solid/table-list
  popular:fa('0 0 576 512','<path fill="currentColor" d="M309.5-18.9c-4.1-8-12.4-13.1-21.4-13.1s-17.3 5.1-21.4 13.1L193.1 125.3 33.2 150.7c-8.9 1.4-16.3 7.7-19.1 16.3s-.5 18 5.8 24.4l114.4 114.5-25.2 159.9c-1.4 8.9 2.3 17.9 9.6 23.2s16.9 6.1 25 2L288.1 417.6 432.4 491c8 4.1 17.7 3.3 25-2s11-14.2 9.6-23.2L441.7 305.9 556.1 191.4c6.4-6.4 8.6-15.8 5.8-24.4s-10.1-14.9-19.1-16.3L383 125.3 309.5-18.9z"/>'), // solid/star
  all:fa('0 0 512 512','<path fill="currentColor" d="M232.5 5.2c14.9-6.9 32.1-6.9 47 0l218.6 101c8.5 3.9 13.9 12.4 13.9 21.8s-5.4 17.9-13.9 21.8l-218.6 101c-14.9 6.9-32.1 6.9-47 0L13.9 149.8C5.4 145.8 0 137.3 0 128s5.4-17.9 13.9-21.8L232.5 5.2zM48.1 218.4l164.3 75.9c27.7 12.8 59.6 12.8 87.3 0l164.3-75.9 34.1 15.8c8.5 3.9 13.9 12.4 13.9 21.8s-5.4 17.9-13.9 21.8l-218.6 101c-14.9 6.9-32.1 6.9-47 0L13.9 277.8C5.4 273.8 0 265.3 0 256s5.4-17.9 13.9-21.8l34.1-15.8zM13.9 362.2l34.1-15.8 164.3 75.9c27.7 12.8 59.6 12.8 87.3 0l164.3-75.9 34.1 15.8c8.5 3.9 13.9 12.4 13.9 21.8s-5.4 17.9-13.9 21.8l-218.6 101c-14.9 6.9-32.1 6.9-47 0L13.9 405.8C5.4 401.8 0 393.3 0 384s5.4-17.9 13.9-21.8z"/>'), // solid/layer-group
@@ -953,8 +951,10 @@ function renderFeeds(){
      ? [sum('unread'),sum('entries'),sum('downloaded')]
      : [f.unread,f.entries,f.downloaded];
    // A group's own row has no error of its own worth mentioning if the OPML itself
-    // reads fine; it is failing when any feed inside it is.
-    const failing=mine.length ? mine.find(c=>c.failing)?.failing : f.failing;
+    // reads fine; it is failing when any feed inside it is. The feed's own page says what
+    // went wrong (failBannerHTML); the list only has to make it findable.
+    const bad=c=>c.failing?.reason||c.last_error;
+    const err=mine.length ? mine.map(bad).find(Boolean) : bad(f);
    const el=document.createElement('div');
    el.className='feed'+(S.feed===f.id?' sel':'')+(depth?' child':'')+(kids?' group':'');
    el.tabIndex=0; el.dataset.id=f.id;
@@ -966,7 +966,7 @@ function renderFeeds(){
      `${mine.length?plural(mine.length,'feed'):plural(eps,'item')} · ${saved} downloaded`+
      `</small></div>`+
      (f.orphaned?'<span class="tag" title="No longer listed, kept because it has downloads">Gone</span>':'')+
-      (failing?`<span class="tag" style="color:var(--bad)" title="${esc(failing.reason)}">Error</span>`:'')+
+      (err?`<span class="tag" style="color:var(--bad)" title="${esc(err)}" aria-label="Error: ${esc(err)}">!</span>`:'')+
      `<span class="badge${unread?'':' zero'}" title="${unread} unread">${unread>999?'999+':unread}</span>`;
    el.onclick=()=>{ selectFeed(f.id); nav(false); };
    if(kids) $('.chev',el).onclick=ev=>{ ev.stopPropagation(); toggleGroup(f.id); };
@@ -1176,13 +1176,19 @@ async function loadEntries(append){
  if(!S.feed || VIEWS[S.feed]?.url) return;
  const p=new URLSearchParams({offset:S.offset,limit:LIMIT,filter:S.filter,sort:S.sort.col,dir:S.sort.dir});
  if(S.q) p.set('q',S.q);
+  const asked=performance.now();
  const r=await api(S.feed===':all' ? `/api/entries?${p}`
    : `/api/feeds/${encodeURIComponent(S.feed)}/entries?${p}`);
  S.total=r.total;
+  for(const e of r.entries){
+    const w=readWrites.get(readKey(e));
+    if(w && !(w.done<asked)) e.read=w.read;
+  }
  let entries = append ? S.entries.concat(r.entries) : r.entries;
  // A background scan finishing refreshes the list from the server, which -- on the Unread
  // tab -- would drop the item you have open the moment reading it took it off the filter.
  // Keep it until you pick a different one; the next refresh after that no longer protects it.
+  if(S.filter==='unread') entries=entries.filter(e=>!e.read||e.guid===S.sel);
  if(!append && S.sel && !entries.some(e=>e.guid===S.sel)){
    const open=S.entries.find(e=>e.guid===S.sel);
    if(open) entries=[open,...entries];
@@ -1302,17 +1308,34 @@ function swapRow(e){
  if(row) row.replaceWith(epEl(e));
 }

+/// Read and unread as this page last set them, and when the server had it. A list asked for
+/// before then answers with the old state: it put the dot back on an item just read, until
+/// the next refresh took it off again (loadEntries).
+const readWrites=new Map();
+const readKey=e=>e.feed_id+'\n'+e.guid;
+function setRead(e,read){
+  e.read=read;
+  const w={read}; readWrites.set(readKey(e),w);
+  return api(`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}/flags`,
+      {method:'POST',body:JSON.stringify({read})})
+    .then(()=>{ w.done=performance.now(); loadFeeds(true); });
+}
+
 /// Opening an item is reading it. The row is redrawn where it stands rather than the list
 /// reloaded, so an item does not vanish from under the pointer on the Unread tab.
 function markRead(e){
  if(e.read) return;
-  e.read=true;
-  api(`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}/flags`,
-      {method:'POST',body:JSON.stringify({read:true})})
-    .then(()=>loadFeeds(true)).catch(err=>{ e.read=false; toast(err.message,true); });
+  setRead(e,true).catch(err=>{ e.read=false; readWrites.delete(readKey(e)); toast(err.message,true); });
 }

 function selectEntry(e){
+  // On the Unread tab the item you were reading goes as you move on, not whenever a refresh
+  // next happens to come along, which left a few read ones in the list for a while.
+  const prev=S.filter==='unread' && S.sel!==e.guid && S.entries.find(x=>x.guid===S.sel);
+  if(prev&&prev.read){
+    S.entries=S.entries.filter(x=>x!==prev); S.total--;
+    $(`#eps .ep[data-guid="${CSS.escape(prev.guid)}"]`)?.remove();
+  }
  S.sel=e.guid;
  markRead(e);
  $$('#eps .ep').forEach(x=>x.classList.toggle('sel', x.dataset.guid===e.guid));
@@ -1454,7 +1477,7 @@ async function epAction(a,e,el,encId){
  try{
    if(a==='play') play(e, encId!=null ? enc : undefined);
    if(a==='flag'){ e.flagged=!e.flagged; await api(path+'/flags',{method:'POST',body:JSON.stringify({flagged:e.flagged})}); redraw(); }
-    if(a==='read'){ e.read=!e.read; await api(path+'/flags',{method:'POST',body:JSON.stringify({read:e.read})}); redraw(); loadFeeds(true); }
+    if(a==='read'){ await setRead(e,!e.read); redraw(); }
    if(a==='get'){
      if(!enc) return;
      await api(`/api/enclosures/${enc.id}/download`,{method:'POST'});
@@ -1493,9 +1516,7 @@ function markPlayed(){
  player.marked=true;
  const e=player.entry;
  if(!e||e.read) return;
-  e.read=true;
-  api(`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}/flags`,
-      {method:'POST',body:JSON.stringify({read:true})}).then(()=>loadFeeds(true)).catch(()=>{});
+  setRead(e,true).catch(()=>{});
 }

 /// Plays one of the item's files in the player bar: the one asked for, or its first playable one.
@@ -2245,10 +2266,6 @@ $('#signout').onclick=async()=>{ await api('/api/logout',{method:'POST'}); locat
 api('/api/me').then(u=>{
  S.me=u;
  $('#who').textContent=u.name+(u.admin?' · admin':'');
-  // The log names every account and every failed sign-in; that alone is the operator's
-  // business, and the server refuses it from anyone else. Settings stays -- it opens the
-  // same modal for everyone, just without the admin-only fields (see prefsModal).
-  if(!u.admin) $('#logs').hidden=true;
 }).catch(()=>{});
 $('#logs').onclick=logsModal;
 $('#feedFilter').oninput=renderFeeds;
@@ -2265,21 +2282,16 @@ $('#feedlist').onkeydown=ev=>{
 };
 $('#burger').onclick=()=>nav(!$('#sidebar').classList.contains('open'));
 $('#scrim').onclick=()=>nav(false);
-// Dark, light, the 2004 Mac app, and following the system. The header button steps through
-// them and Settings offers the same four as a dropdown; either one sets ipx.theme and both
-// read it, so they never disagree. Auto last in the cycle keeps the existing three clicks in
-// "steps through dark, light and classic" reaching classic exactly where that test expects.
+// Dark, light, the 2004 Mac app, and following the system, chosen in Settings and kept in
+// ipx.theme.
 const THEMES={dark:'Dark',light:'Light',classic:'Classic, the 2004 Mac app',auto:'Auto (matches your system)'};
 const THEME_ORDER=Object.keys(THEMES);
-const nextTheme=t=>THEME_ORDER[(THEME_ORDER.indexOf(t)+1)%THEME_ORDER.length];
 function setTheme(t){
  if(!THEMES[t]) t='dark';
  document.documentElement.dataset.theme=t;
-  $('#theme').title=`Theme: ${THEMES[t]}. Click for ${THEMES[nextTheme(t)]}`;
  const sel=$('#stheme'); if(sel) sel.value=t;
  try{ localStorage.setItem('ipx.theme',t); }catch{}
 }
-$('#theme').onclick=()=>setTheme(nextTheme(document.documentElement.dataset.theme));
 try{ setTheme(localStorage.getItem('ipx.theme')); }catch{ setTheme('dark'); }

 /* ---------------- live events ---------------- */
@@ -2319,7 +2331,9 @@ function connect(){
      if(ev.new){ fresh[ev.feed]=(fresh[ev.feed]||0)+ev.new; tellNew(); }
      refreshFeeds(); if(ev.feed===S.feed||S.feed===':all') refreshEntries();
    }
-    else if(ev.ev==='feed_error'){ toast(ev.feed+': '+ev.msg,true); refreshFeeds(); }
+    // No toast: a scan of every feed raised one per failure, to everyone. The feed list's
+    // red ! marks the feed instead, and its page says why.
+    else if(ev.ev==='feed_error') refreshFeeds();
    else if(ev.ev==='scan_done'){ refreshFeeds(); refreshEntries(); }
  };
  sse.onerror=()=>{ sse.close(); setTimeout(connect,4000); };