fix(library): stop offering the episode you just finished as "up next"
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 16m0s
🏗️ Build and Test JellyTau / Supply Chain (push) Successful in 35s
Traceability Validation / Check Requirement Traces (push) Successful in 26s
🏗️ Build and Test JellyTau / Android Compile Check (push) In progress
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 16m0s
🏗️ Build and Test JellyTau / Supply Chain (push) Successful in 35s
Traceability Validation / Check Requirement Traces (push) Successful in 26s
🏗️ Build and Test JellyTau / Android Compile Check (push) In progress
Finish an episode, leave the player with Back, and the season view still put the yellow ring and the "Up next" badge on the episode that had just ended -- and scrolled to it. Nothing records completion locally. storage_update_playback_progress writes a position and never touches is_played, and mirror_user_data carried the server's favourite flag and position but not its played flag, so that column was written by nothing except an explicit local toggle. By the second visit to a series page get_items is a cache hit, so every episode reads back unwatched; meanwhile Jellyfin's Next Up is still one stop-report behind and names the episode that just ended. pick_current_episode had no reason to disagree with either of them. is_finished -- the played flag, or a position at or past MAX_PROGRESS_FRACTION of the runtime, the same 95% threshold that already disqualifies an episode from counting as in-progress -- replaces the bare is_played in the furthest-watched scan and the first-unwatched fallback, and screens the Next Up candidate before it is accepted: the server is briefly behind, the local position is not. The current episode becomes the next one, and the highlight, the badge, the auto-scroll and which season starts expanded all follow it. mirror_user_data now carries is_played alongside the rest, under the same pending_sync = 0 conflict rule, so watched state survives a cache write instead of being dropped -- which is also what puts the checkmarks back in the season list. TRACES: UR-025, UR-062 | DR-264 | UT-239, UT-240
This commit is contained in:
@@ -457,6 +457,7 @@ Internal architecture, components, and application logic.
|
||||
| DR-261 | Subtitles are drawn **over the picture, not on a black bar across it**. `SubtitleView.setUserDefaultStyle()` reads Android's captioning preferences and falls back to media3's `DEFAULT` when the viewer has set none — and that default is white on opaque black, so every line arrived in a box as wide as the text. The viewer's own style is kept and only the two colours that paint a box, background and window, are cleared: someone who has configured captions in accessibility settings has said something specific about colour, typeface and edges, and replacing all of it to remove a background would answer a question they did not ask. A style specifying no edge gets a black outline, because without a box the text must supply its own contrast or it is unreadable over a bright scene; a style that already names an edge keeps it | Player | UR-020 | Done (pending device verification) |
|
||||
| DR-262 | The A-Z jump strip is bounded by the **scroller it lives in**, not by the viewport minus a guess at the bottom bars. `AlphabetScrollBar` sized itself as `window.innerHeight` minus a hardcoded `bottomGap` — 5rem, 7rem or 11rem, picked by platform and whether the mini player was showing — which dates from when the mini player and bottom nav were `position: fixed` overlays. They have been in-flow flex siblings below the scroller since BottomUi (DR-009), so the scroller's own bottom edge *is* the top of the mini player and can simply be measured. The guess was short on every device with a navigation or gesture bar, because `--safe-bottom` is padded *inside* BottomUi (DR-112) and no guess knew about it: the strip overran the scrollport by ~45px with the nav alone, ~18px with the mini player and ~50px in remote mode, burying one to three letters where they could not be tapped. The ancestor is resolved by computed `overflow-y` rather than `closest("main")`, since the root shell scrolls in a plain `<div>` and a miss silently fell back to the viewport — reinstating the bug on any route outside `/library`. Observing the scroller for resize is also what makes the mini player appearing re-measure, so the component no longer subscribes to player or platform stores at all | UI | UR-007 | Done |
|
||||
| DR-263 | Autoplay crosses the **season boundary**. `fetch_next_episode_for_item` listed the episodes of the current season and stopped dead at the last one, so the end of a season produced `AutoplayDecision::Stop`. On the Android background-audio handoff (UR-040) that is felt as playback simply pausing mid-binge with the screen locked and no UI to un-pause it — the same end that mid-season advances through in the backend. The lookup now walks the series' seasons, sorted client-side by index number because the offline repository ignores `sort_by`, and takes the first episode of the next season that has any, skipping empty ones. Specials are never rolled *into*: Jellyfin numbers them 0 so they sort ahead of season 1, but a server that leaves the index unset sorts them last, exactly where the walk would land. The lookup sits below the sleep-timer gate in `on_playback_ended`, so a timer set to end-of-episode or a remaining-episode count still stops at the boundary rather than being carried past it | Player | UR-023, UR-040 | Done |
|
||||
| DR-264 | The episode a viewer *just finished* is no longer offered as the one they are up to. Nothing records completion locally: the stop report writes a position through `storage_update_playback_progress` (which never sets `is_played`), and the cache mirror carried the server's flag not at all — so on a cache hit every episode read back as unwatched. Leaving the player with Back reloads the series page within a second of the stop report, inside the window where Jellyfin's Next Up still names the episode that just ended, and `pick_current_episode` handed it straight back: the season view kept the yellow ring and the "Up next" badge on the episode the viewer had just watched, and scrolled to it. Two halves. (a) `is_finished` — the played flag **or** a position at or past `MAX_PROGRESS_FRACTION` of the runtime, the same 95% threshold that already disqualifies an episode from counting as in-progress — replaces the bare `is_played` in the furthest-watched scan and the first-unwatched fallback, and screens the Next Up candidate: the server is one stop-report behind for a moment, the local position is not. (b) `OfflineRepository::mirror_user_data` carries `is_played` alongside the favourite flag and the position, under the same `pending_sync = 0` conflict rule, so watched state survives a cache write instead of being dropped — that flag was previously written by nothing but an explicit local toggle | Repository | UR-062 | Done |
|
||||
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
|
||||
|
||||
---
|
||||
@@ -787,6 +788,8 @@ Internal architecture, components, and application logic.
|
||||
| UT-236 | The strip still fills the space it does have rather than stopping a letter short | DR-262 | Done |
|
||||
| UT-237 | The strip falls back to the viewport with no scroll container, and never computes a negative height when scrolled past its floor | DR-262 | Done |
|
||||
| UT-238 | The last episode of a season rolls over into the first of the next, skips an empty season on the way, and still stops there when the sleep timer says so | DR-263 | Done |
|
||||
| UT-239 | An episode watched to the end is not the current episode: not when the server's Next Up still names it (the stale answer is skipped in favour of the one after it), and not offline, where it counts as watched in the furthest-watched scan | DR-264 | Done |
|
||||
| UT-240 | Caching a server result mirrors its played flag locally — as synced, never invented for an item that carries no user data, and never over an unsynced local toggle | DR-264 | Done |
|
||||
|
||||
### Integration Tests
|
||||
|
||||
|
||||
+7769
-6916
File diff suppressed because it is too large
Load Diff
@@ -684,26 +684,33 @@ impl OfflineRepository {
|
||||
/// The position half is what makes cross-device resume work: the resume
|
||||
/// check reads this table alone, so before it was mirrored an item watched
|
||||
/// elsewhere resumed from whatever *this* device last saw, or not at all.
|
||||
/// The played flag rides along for the same reason: nothing else writes it
|
||||
/// but an explicit local toggle, so a cached episode list read every
|
||||
/// episode back as unwatched — the list the season view ticks and the one
|
||||
/// `pick_current_episode` reads to decide what is up next (DR-264).
|
||||
///
|
||||
/// TRACES: UR-025, UR-069 | DR-114, DR-155 | UT-102, UT-152
|
||||
/// TRACES: UR-025, UR-062, UR-069 | DR-114, DR-155, DR-264 | UT-102, UT-152, UT-240
|
||||
async fn mirror_user_data(&self, item: &MediaItem, now: &str) -> Result<(), RepoError> {
|
||||
let user_data = item.user_data.as_ref();
|
||||
let is_favorite = user_data.and_then(|ud| ud.is_favorite);
|
||||
let position_ticks = user_data.and_then(|ud| ud.playback_position_ticks);
|
||||
let is_played = user_data.and_then(|ud| ud.is_played);
|
||||
|
||||
// Nothing the server actually told us about — do not invent a row.
|
||||
if is_favorite.is_none() && position_ticks.is_none() {
|
||||
if is_favorite.is_none() && position_ticks.is_none() && is_played.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let query = Query::with_params(
|
||||
"INSERT INTO user_data
|
||||
(user_id, item_id, is_favorite, playback_position_ticks, synced_at, pending_sync)
|
||||
VALUES (?1, ?2, ?3, ?4, ?5, 0)
|
||||
(user_id, item_id, is_favorite, playback_position_ticks, is_played,
|
||||
synced_at, pending_sync)
|
||||
VALUES (?1, ?2, ?3, ?4, ?5, ?6, 0)
|
||||
ON CONFLICT(user_id, item_id) DO UPDATE SET
|
||||
is_favorite = COALESCE(excluded.is_favorite, user_data.is_favorite),
|
||||
playback_position_ticks = COALESCE(
|
||||
excluded.playback_position_ticks, user_data.playback_position_ticks),
|
||||
is_played = COALESCE(excluded.is_played, user_data.is_played),
|
||||
synced_at = excluded.synced_at
|
||||
WHERE user_data.pending_sync = 0",
|
||||
vec![
|
||||
@@ -715,6 +722,9 @@ impl OfflineRepository {
|
||||
position_ticks
|
||||
.map(QueryParam::Int64)
|
||||
.unwrap_or(QueryParam::Null),
|
||||
is_played
|
||||
.map(|p| QueryParam::Int(if p { 1 } else { 0 }))
|
||||
.unwrap_or(QueryParam::Null),
|
||||
QueryParam::String(now.to_string()),
|
||||
],
|
||||
);
|
||||
@@ -4786,6 +4796,98 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// UT-240 — the server's *played* flag is mirrored locally, so an episode
|
||||
/// watched anywhere is watched here.
|
||||
///
|
||||
/// The mirror carried only the favourite flag and the position, so
|
||||
/// `user_data.is_played` was written by nothing but an explicit local
|
||||
/// toggle: a cached episode list reported every episode as unwatched, which
|
||||
/// is the list `pick_current_episode` reads to decide what is up next
|
||||
/// (DR-264), and the list the season view ticks.
|
||||
///
|
||||
/// TRACES: UR-025, UR-062 | DR-264 | UT-240
|
||||
#[tokio::test]
|
||||
async fn test_save_to_cache_mirrors_played_flag_without_clobbering_pending() {
|
||||
use crate::storage::db_service::DatabaseService;
|
||||
let db_service = create_test_db();
|
||||
let repo = OfflineRepository::new(
|
||||
db_service.clone(),
|
||||
"test-server".to_string(),
|
||||
"test-user".to_string(),
|
||||
);
|
||||
|
||||
let played_flag = |id: &'static str| {
|
||||
let db = db_service.clone();
|
||||
async move {
|
||||
db.query_optional(
|
||||
Query::with_params(
|
||||
"SELECT is_played, pending_sync FROM user_data \
|
||||
WHERE user_id = ? AND item_id = ?",
|
||||
vec![
|
||||
QueryParam::String("test-user".to_string()),
|
||||
QueryParam::String(id.to_string()),
|
||||
],
|
||||
),
|
||||
|row| Ok((row.get::<_, Option<i32>>(0)?, row.get::<_, Option<i32>>(1)?)),
|
||||
)
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
};
|
||||
|
||||
// Watched to the end on another client.
|
||||
let mut watched = create_test_item("ep-4", "Watched Elsewhere", None);
|
||||
watched.user_data = Some(UserData {
|
||||
is_played: Some(true),
|
||||
..Default::default()
|
||||
});
|
||||
// No user data at all — must not fabricate an "unwatched" record.
|
||||
let untouched = create_test_item("ep-5", "No User Data", None);
|
||||
|
||||
repo.save_to_cache("parent-1", &[watched, untouched])
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
played_flag("ep-4").await,
|
||||
Some((Some(1), Some(0))),
|
||||
"the server's played flag should be mirrored as synced"
|
||||
);
|
||||
assert_eq!(
|
||||
played_flag("ep-5").await,
|
||||
None,
|
||||
"an item without UserData should not get an invented played flag"
|
||||
);
|
||||
|
||||
// Marked unwatched here while the server was unreachable.
|
||||
db_service
|
||||
.execute(Query::with_params(
|
||||
"UPDATE user_data SET is_played = 0, pending_sync = 1 \
|
||||
WHERE user_id = ? AND item_id = ?",
|
||||
vec![
|
||||
QueryParam::String("test-user".to_string()),
|
||||
QueryParam::String("ep-4".to_string()),
|
||||
],
|
||||
))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let mut still_played = create_test_item("ep-4", "Watched Elsewhere", None);
|
||||
still_played.user_data = Some(UserData {
|
||||
is_played: Some(true),
|
||||
..Default::default()
|
||||
});
|
||||
repo.save_to_cache("parent-1", &[still_played])
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
played_flag("ep-4").await,
|
||||
Some((Some(0), Some(1))),
|
||||
"an unsynced local toggle must survive a cache write"
|
||||
);
|
||||
}
|
||||
|
||||
/// UT-152 — a server item carrying *only* a position (no favourite flag)
|
||||
/// still gets mirrored.
|
||||
///
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
//! I/O half (`fetch_series_episodes`, `resolve_current_episode`) so the policy
|
||||
//! can be unit-tested without standing up a repository.
|
||||
//!
|
||||
//! TRACES: UR-062 | DR-101
|
||||
//! TRACES: UR-062 | DR-101, DR-264 | UT-239
|
||||
|
||||
use super::{GetItemsOptions, MediaItem, MediaRepository, RepoError};
|
||||
|
||||
@@ -83,6 +83,34 @@ fn is_played(item: &MediaItem) -> bool {
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Has the viewer reached the end of this episode?
|
||||
///
|
||||
/// The played *flag* is not enough. Nothing records completion locally — the
|
||||
/// stop report writes a position, and the cache mirror carries the server's
|
||||
/// flag only on the next refresh — so within seconds of an episode ending the
|
||||
/// only local evidence that it is over is its position, parked at the very end
|
||||
/// of its runtime. Leaving the player with Back reloads the series page inside
|
||||
/// that window (DR-264).
|
||||
fn is_finished(item: &MediaItem) -> bool {
|
||||
if is_played(item) {
|
||||
return true;
|
||||
}
|
||||
|
||||
let Some(user_data) = item.user_data.as_ref() else {
|
||||
return false;
|
||||
};
|
||||
let position_ms = user_data
|
||||
.playback_position_ms
|
||||
.or_else(|| user_data.playback_position_ticks.map(|t| t / 10_000))
|
||||
.unwrap_or(0);
|
||||
// Without a duration a position says nothing about how much is left.
|
||||
let Some(duration_ms) = item.duration_ms.filter(|d| *d > 0) else {
|
||||
return false;
|
||||
};
|
||||
|
||||
position_ms as f64 / duration_ms as f64 >= MAX_PROGRESS_FRACTION
|
||||
}
|
||||
|
||||
fn belongs_to_series(item: &MediaItem, series_id: &str) -> bool {
|
||||
item.series_id.as_deref() == Some(series_id)
|
||||
}
|
||||
@@ -130,11 +158,20 @@ pub fn pick_current_episode(
|
||||
return Some(found.clone());
|
||||
}
|
||||
|
||||
// 2. Next Up for this series.
|
||||
if let Some(found) = next_up
|
||||
.iter()
|
||||
.find(|e| e.series_id.is_none() || belongs_to_series(e, series_id))
|
||||
{
|
||||
// 2. Next Up for this series — unless it names an episode we can already
|
||||
// see is over. Next Up is the server's answer, and the server is one
|
||||
// stop-report behind for a moment after an episode ends; the local
|
||||
// position is not, so a finished candidate is dropped rather than handed
|
||||
// back as "up next" (DR-264).
|
||||
if let Some(found) = next_up.iter().find(|e| {
|
||||
if !(e.series_id.is_none() || belongs_to_series(e, series_id)) {
|
||||
return false;
|
||||
}
|
||||
// Judge it by the copy from `episodes` when there is one: that is the
|
||||
// copy carrying the local user-data.
|
||||
let local = episodes.iter().find(|listed| listed.id == e.id);
|
||||
!is_finished(local.unwrap_or(e))
|
||||
}) {
|
||||
// Prefer the copy from `episodes` when we have one: it carries the
|
||||
// user-data and images the list already fetched.
|
||||
let matched = episodes.iter().find(|e| e.id == found.id);
|
||||
@@ -145,7 +182,7 @@ pub fn pick_current_episode(
|
||||
// unwatched: a viewer who skipped the pilot but is deep into season 3
|
||||
// must not be dragged back to S1E1. An earlier gap is a deliberate skip;
|
||||
// where they stopped is the *last* thing they watched.
|
||||
if let Some(furthest) = episodes.iter().rposition(is_played) {
|
||||
if let Some(furthest) = episodes.iter().rposition(is_finished) {
|
||||
if let Some(found) = episodes.get(furthest + 1) {
|
||||
return Some(found.clone());
|
||||
}
|
||||
@@ -153,7 +190,7 @@ pub fn pick_current_episode(
|
||||
|
||||
// Nothing watched yet (or the furthest-watched episode is the finale):
|
||||
// the first unwatched episode in series order.
|
||||
if let Some(found) = episodes.iter().find(|e| !is_played(e)) {
|
||||
if let Some(found) = episodes.iter().find(|e| !is_finished(e)) {
|
||||
return Some(found.clone());
|
||||
}
|
||||
|
||||
@@ -427,6 +464,41 @@ mod tests {
|
||||
assert_eq!(current.id, "s2e1");
|
||||
}
|
||||
|
||||
/// The episode the viewer just finished must not still be "up next".
|
||||
///
|
||||
/// Leaving the player with Back reloads the series page within a second of
|
||||
/// the stop report, and Jellyfin's Next Up can still name the episode that
|
||||
/// just ended. Locally we know better: the position sits at the very end of
|
||||
/// its runtime.
|
||||
///
|
||||
/// TRACES: UR-062 | DR-264 | UT-239
|
||||
#[test]
|
||||
fn a_just_finished_episode_is_not_current_even_when_next_up_still_names_it() {
|
||||
let mut eps = season(1, 5);
|
||||
eps[0] = watched(eps[0].clone());
|
||||
// Just finished: the position is at the end, the flag has not landed.
|
||||
eps[1] = in_progress(eps[1].clone(), 0.99);
|
||||
// The server has not caught up with the stop report.
|
||||
let next_up = vec![episode("s1e2", 1, 2)];
|
||||
|
||||
let current = pick_current_episode(SERIES, &eps, &next_up, &[]).unwrap();
|
||||
assert_eq!(current.id, "s1e3");
|
||||
}
|
||||
|
||||
/// The same, offline: with no Next Up to lean on, an episode watched to the
|
||||
/// end counts as watched when scanning for the furthest-watched one.
|
||||
///
|
||||
/// TRACES: UR-062 | DR-264 | UT-239
|
||||
#[test]
|
||||
fn an_episode_watched_to_the_end_counts_as_watched_offline() {
|
||||
let mut eps = season(1, 5);
|
||||
eps[0] = watched(eps[0].clone());
|
||||
eps[1] = in_progress(eps[1].clone(), 0.99);
|
||||
|
||||
let current = pick_current_episode(SERIES, &eps, &[], &[]).unwrap();
|
||||
assert_eq!(current.id, "s1e3");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_never_watched_series_opens_on_its_premiere() {
|
||||
let eps = [season(2, 3), season(1, 3)].concat();
|
||||
|
||||
Reference in New Issue
Block a user