From 2f3b22e2d578ade6c0b24627161ebe360ddcf993 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 06:55:29 +0200 Subject: [PATCH 1/2] feat: download a track from its context menu Step 3 of RFC-012 (decision 5). Download is the stream ticket: GET /api/v2/stream/{ticket}?download=1 answers the same original bytes, through the same path guard and the same re-check of access, with Content-Disposition: attachment. No new ticket kind, route or read path. - The name is the file's basename for a scanned track, and "Artist - Title.ext" for one received through RFC-008, whose basename is its hash. It is sent in both RFC 6266 forms: an ASCII fallback with quotes, backslashes, separators, controls and non-ASCII replaced, and filename* in percent-encoded UTF-8. - A download is the file, so asking for one with a transcode, a bit rate or an offset answers 422 rather than an attachment that is not it. - The web client's track menu gains Download: it mints a ticket and saves from it through a download link, and says so when the ticket cannot be minted rather than doing nothing. - The menu and that alert are portalled to the body: a page block rising into place carries a transform, which made it the containing block of their position: fixed and put the alert under the page header. - RFC-012's "Implemented by" line names #298 and this change. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-012-library-browser.md | 8 +- src/media.rs | 161 ++++++++++++++++++++++++++- src/services/mod.rs | 2 + tests/media.rs | 53 +++++++++ webapp/e2e/studio-nocturne.spec.ts | 76 +++++++++++++ webapp/src/components/icons.tsx | 11 +- webapp/src/components/track-menu.tsx | 80 +++++++++++-- webapp/src/lib/download.test.ts | 17 +++ webapp/src/lib/download.ts | 23 ++++ webapp/src/state/i18n.tsx | 7 ++ webapp/src/styles/styles.css | 32 ++++++ 11 files changed, 452 insertions(+), 18 deletions(-) create mode 100644 webapp/src/lib/download.test.ts create mode 100644 webapp/src/lib/download.ts diff --git a/docs/rfcs/RFC-012-library-browser.md b/docs/rfcs/RFC-012-library-browser.md index 4259b48a..4a0d708d 100644 --- a/docs/rfcs/RFC-012-library-browser.md +++ b/docs/rfcs/RFC-012-library-browser.md @@ -1,8 +1,12 @@ # RFC-012 — The library browser: folders as a view, filing as metadata - **Status**: Proposed -- **Implemented by**: not started. The *Status* field above never flips in this - project; this line is the one that will say what is running. +- **Implemented by**: in progress, in the order of "The order of the work" + below: [#298](https://github.com/InstaZDLL/waveflow-server/pull/298) (the + context menu and Properties, with `relative_path` on the track) and the pull + request adding download (`?download=1` on the stream ticket). The *Status* + field above never flips in this project; this line is the one that says what + is running. - **Date**: 2026-10-21 - **Revised**: 2026-10-21, after review on #297: the **Received** folder's contract, the depth check on a move, the folder cap that bounds the tree's diff --git a/src/media.rs b/src/media.rs index 028fa46e..c67e3e94 100644 --- a/src/media.rs +++ b/src/media.rs @@ -91,6 +91,22 @@ pub struct StreamQuery { pub offset_ms: u64, } +/// `?download=1` on a stream ticket (RFC-012 decision 5). +/// +/// Its own extractor rather than a field on [`StreamQuery`], which the +/// Subsonic façade and the bearer route share and which has no business +/// learning a header choice only the ticket route makes. +#[derive(Debug, Default, Deserialize)] +pub struct DownloadQuery { + download: Option, +} + +impl DownloadQuery { + fn asked(&self) -> bool { + matches!(self.download.as_deref(), Some("1" | "true")) + } +} + #[derive(Debug)] pub enum MediaError { Unauthorized, @@ -1190,13 +1206,15 @@ pub async fn create_stream_ticket( ("ticket" = String, Path), ("format" = Option, Query, description = "raw, mp3 or opus"), ("bitrate" = Option, Query), - ("offset_ms" = Option, Query) + ("offset_ms" = Option, Query), + ("download" = Option, Query, description = "1: the original, as an attachment named after the file (RFC-012)") ), responses( (status = 200, description = "Original or live transcoded audio"), (status = 206, description = "Original or completed cached transcode range"), (status = 404), (status = 416), + (status = 422, description = "A download asked for together with a transcode or an offset"), (status = 429) ) )] @@ -1204,6 +1222,7 @@ pub async fn stream_with_ticket( State(state): State, AxumPath(ticket): AxumPath, Query(query): Query, + Query(download): Query, headers: HeaderMap, ) -> Result { // An invalid, forged or expired ticket is indistinguishable from an unknown @@ -1226,9 +1245,118 @@ pub async fn stream_with_ticket( let range = headers .get(header::RANGE) .and_then(|value| value.to_str().ok()); - state.media.serve(user_id, track, query, range).await + if !download.asked() { + return state.media.serve(user_id, track, query, range).await; + } + // A download is the file. A transcode, a bit rate or an offset would make + // it something else under the file's own name, so asking for one with it + // is refused rather than quietly ignored. + if query.format != OutputFormat::Raw || query.bitrate.is_some() || query.offset_ms != 0 { + return Err(MediaError::InvalidRequest); + } + let name = download_name(&state, user_id, &track).await; + let mut response = state.media.serve(user_id, track, query, range).await?; + response + .headers_mut() + .insert(header::CONTENT_DISPOSITION, attachment(&name)); + Ok(response) +} + +/// The name a downloaded file is saved under. +/// +/// A scanned track's own basename, which is what its owner called it. A track +/// received through RFC-008 is named by its hash on disk, so it is saved as +/// `Artist - Title.ext` from its tags instead — the hash is truthful and of no +/// use to anyone who opens their downloads folder. +async fn download_name(state: &AppState, user_id: Uuid, track: &StreamTrack) -> String { + let path = Path::new(&track.relative_path); + let basename = path + .file_name() + .and_then(OsStr::to_str) + .unwrap_or("track") + .to_owned(); + let received = track + .relative_path + .split('/') + .next() + .is_some_and(|first| first == crate::services::MANAGED_DIR); + if !received { + return basename; + } + let extension = path.extension().and_then(OsStr::to_str).unwrap_or(""); + let song = state + .services + .songs_by_ids(user_id, &[track.id]) + .await + .ok() + .and_then(|songs| songs.into_iter().next()); + let Some(song) = song else { + return basename; + }; + let stem = match song + .artist + .as_deref() + .map(str::trim) + .filter(|a| !a.is_empty()) + { + Some(artist) => format!("{artist} - {}", song.title.trim()), + None => song.title.trim().to_owned(), + }; + if stem.is_empty() { + return basename; + } + if extension.is_empty() { + stem + } else { + format!("{stem}.{extension}") + } +} + +/// `attachment` with the name in both forms RFC 6266 gives it. +/// +/// `filename` is an ASCII fallback, every character a browser could misread +/// in a quoted string — a quote, a backslash, a control, anything outside +/// ASCII — replaced by `_`, and a separator too, so no browser builds a path +/// from it. `filename*` carries the real name, percent-encoded UTF-8, and +/// every current browser prefers it. +fn attachment(name: &str) -> HeaderValue { + let cleaned: String = name + .chars() + .map(|c| match c { + '/' | '\\' => '_', + c if c.is_control() => '_', + c => c, + }) + .collect(); + let fallback: String = cleaned + .chars() + .map(|c| match c { + ' '..='~' if c != '"' && c != '\\' => c, + _ => '_', + }) + .collect(); + let encoded = percent_encoding::utf8_percent_encode(&cleaned, ATTR_CHAR).to_string(); + HeaderValue::from_str(&format!( + "attachment; filename=\"{fallback}\"; filename*=UTF-8''{encoded}" + )) + .unwrap_or_else(|_| HeaderValue::from_static("attachment")) } +/// Everything but RFC 5987's `attr-char` is percent-encoded in `filename*`. +const ATTR_CHAR: &percent_encoding::AsciiSet = &percent_encoding::NON_ALPHANUMERIC + .remove(b'!') + .remove(b'#') + .remove(b'$') + .remove(b'&') + .remove(b'+') + .remove(b'-') + .remove(b'.') + .remove(b'^') + .remove(b'_') + .remove(b'`') + .remove(b'|') + .remove(b'~'); + #[utoipa::path( get, path = "/api/v2/tracks/{track_id}/stream", @@ -1661,10 +1789,35 @@ mod tests { use tokio::sync::{Mutex, Semaphore}; use super::{ - cache_key, parse_range, prune_cache, secure_track_path, starts_at_the_first_byte, - MediaInner, MediaService, OutputFormat, + attachment, cache_key, parse_range, prune_cache, secure_track_path, + starts_at_the_first_byte, MediaInner, MediaService, OutputFormat, }; + /// A download's name in both forms, whatever it holds. + /// + /// The fallback is what an old browser builds a path from, so a quote, a + /// backslash, a separator or anything outside ASCII must not survive in + /// it; `filename*` must carry the real name, accents and all. + #[test] + fn a_download_name_is_safe_in_both_forms() { + let header = attachment("Björk - Hyperballad \"live\" 1/2\\3.flac"); + let header = header.to_str().unwrap(); + assert_eq!( + header, + "attachment; filename=\"Bj_rk - Hyperballad _live_ 1_2_3.flac\"; \ + filename*=UTF-8''Bj%C3%B6rk%20-%20Hyperballad%20%22live%22%201_2_3.flac" + ); + // A control character is no part of a name. + let header = attachment("a\nb.flac"); + assert!( + header + .to_str() + .unwrap() + .starts_with("attachment; filename=\"a_b.flac\""), + "{header:?}" + ); + } + #[test] fn ranges_are_bounded_and_validated() { assert_eq!(parse_range("bytes=2-5", 10), Some((2, 5))); diff --git a/src/services/mod.rs b/src/services/mod.rs index 4c85cb64..97714e75 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -1375,6 +1375,8 @@ mod songs; mod sync; mod track_metadata; mod uploads; +/// Where received files land in a library, for the download route's naming. +pub(crate) use uploads::MANAGED_DIR; /// Crate-visible only: the adapter builds the browser-less journey's address /// from it, and nothing outside this binary has a use for it. diff --git a/tests/media.rs b/tests/media.rs index 3c502384..0153f88d 100644 --- a/tests/media.rs +++ b/tests/media.rs @@ -712,6 +712,59 @@ async fn stream_tickets_authorise_browser_playback_without_a_bearer() { .unwrap(); assert_eq!(ranged.status(), StatusCode::PARTIAL_CONTENT); + // Playing sends no attachment header; that is a download's alone. + assert!(played.headers().get("content-disposition").is_none()); + let played_length = played.headers()["content-length"].clone(); + + // `?download=1` is the same file, saved under the file's own name + // (RFC-012 decision 5) — no other route, no other read. + let basename = tracks[0]["relative_path"] + .as_str() + .unwrap() + .rsplit('/') + .next() + .unwrap() + .to_owned(); + let downloaded = router + .clone() + .oneshot( + Request::get(format!("{url}?download=1")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(downloaded.status(), StatusCode::OK); + assert_eq!(downloaded.headers()["content-length"], played_length); + let disposition = downloaded.headers()["content-disposition"] + .to_str() + .unwrap() + .to_owned(); + assert!( + disposition.starts_with("attachment; filename=\"") + && disposition.contains(&format!("filename*=UTF-8''{basename}")), + "{disposition}" + ); + + // A download is the file: asked for with a transcode, an offset or a bit + // rate, it is refused rather than handed over under the file's name. + for refused in [ + format!("{url}?download=1&format=mp3"), + format!("{url}?download=1&offset_ms=1000"), + format!("{url}?download=1&bitrate=128"), + ] { + let response = router + .clone() + .oneshot(Request::get(&refused).body(Body::empty()).unwrap()) + .await + .unwrap(); + assert_eq!( + response.status(), + StatusCode::UNPROCESSABLE_ENTITY, + "{refused}" + ); + } + // An expired ticket is refused even though it is otherwise well formed. let expired = waveflow_server::stream_ticket::mint( &secret_box, diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index fa0589c3..380b7bd7 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -1419,6 +1419,7 @@ test("opens a track's properties from its context menu", async ({ page }) => { await expect(menu.getByRole("menuitem")).toHaveText([ "Play", "Add to queue", + "Download", "Properties", "Edit tags", ]); @@ -1430,6 +1431,7 @@ test("opens a track's properties from its context menu", async ({ page }) => { ); await page.keyboard.press("ArrowDown"); await page.keyboard.press("ArrowDown"); + await page.keyboard.press("ArrowDown"); await expect(menu.getByRole("menuitem", { name: "Properties" })).toBeFocused(); await page.keyboard.press("Enter"); @@ -1458,6 +1460,7 @@ test("opens a track's properties from its context menu", async ({ page }) => { await page.keyboard.press("Shift+F10"); await page.keyboard.press("ArrowDown"); await page.keyboard.press("ArrowDown"); + await page.keyboard.press("ArrowDown"); await page.keyboard.press("Enter"); await expect(panel).toBeVisible(); await page.mouse.click(2, 2); @@ -1470,6 +1473,79 @@ test("opens a track's properties from its context menu", async ({ page }) => { await expect(menu).toHaveCount(0); }); +/** + * Download is the stream ticket with `?download=1` (RFC-012 decision 5). + * + * The browser cannot send the session on a navigation, so the menu mints a + * ticket and saves from it through a link with `download` and no value, + * which leaves the file's name to the server's `Content-Disposition`. That + * header is asserted server-side, against a real file; what is asserted here + * is the link the page builds. Chromium hands a `download` link straight to + * its download manager, past every route a test could install, so the link + * is caught as it is clicked rather than on the wire. + */ +test("downloads a track from its context menu", async ({ page }) => { + await page.route("**/api/v2/songs?*", (route) => + route.fulfill({ json: [correctable.song] }), + ); + await page.route("**/api/v2/tracks/song-9/stream-ticket", (route) => + route.fulfill({ + json: { url: "/api/v2/stream/ticket-9", expires_at: Date.now() + 60_000 }, + }), + ); + await page.goto("/songs"); + await page.evaluate(() => { + const seen = window as unknown as { + __saved: Array<{ href: string | null; download: string }>; + }; + seen.__saved = []; + HTMLAnchorElement.prototype.click = function (this: HTMLAnchorElement) { + seen.__saved.push({ + href: this.getAttribute("href"), + download: this.getAttribute("download") ?? "absent", + }); + }; + }); + await page + .locator(".songs tbody tr") + .first() + .locator("button.song-title") + .focus(); + await page.keyboard.press("Shift+F10"); + const menu = page.getByRole("menu", { name: "Actions for Army Of Me" }); + await menu.getByRole("menuitem", { name: "Download" }).click(); + await expect(menu).toHaveCount(0); + await expect + .poll(() => + page.evaluate( + () => + (window as unknown as { __saved: unknown[] }).__saved, + ), + ) + .toEqual([{ href: "/api/v2/stream/ticket-9?download=1", download: "" }]); + // Nothing is left in the document once the link has been followed. + await expect(page.locator("a[download]")).toHaveCount(0); + + // A ticket that cannot be minted says so, rather than a click doing + // nothing; and saves nothing. + await page.route("**/api/v2/tracks/song-9/stream-ticket", (route) => + route.fulfill({ status: 503, body: "" }), + ); + await page.keyboard.press("Shift+F10"); + await menu.getByRole("menuitem", { name: "Download" }).click(); + const failed = page + .getByRole("alert") + .filter({ hasText: "The download could not start" }); + await expect(failed).toHaveText("The download could not start. Try again."); + await failed.getByRole("button", { name: "Dismiss" }).click(); + await expect(failed).toHaveCount(0); + expect( + await page.evaluate( + () => (window as unknown as { __saved: unknown[] }).__saved.length, + ), + ).toBe(1); +}); + test("gives random playback one clear primary action", async ({ page }) => { await page.route("**/api/v2/songs/random?*", (route) => route.fulfill({ json: [track] }), diff --git a/webapp/src/components/icons.tsx b/webapp/src/components/icons.tsx index e5681a88..953fc483 100644 --- a/webapp/src/components/icons.tsx +++ b/webapp/src/components/icons.tsx @@ -29,9 +29,18 @@ export type IconName = | "close" | "upload" | "radio" - | "info"; + | "info" + | "download"; const paths: Record = { + // An arrow out of the library onto a tray: the file, saved. + download: ( + <> + + + + + ), // A circled i: what a track is, read and not changed. info: ( <> diff --git a/webapp/src/components/track-menu.tsx b/webapp/src/components/track-menu.tsx index 42016409..7fd6a200 100644 --- a/webapp/src/components/track-menu.tsx +++ b/webapp/src/components/track-menu.tsx @@ -8,8 +8,10 @@ import { useRef, useState, } from "react"; +import { createPortal } from "react-dom"; -import type { Song } from "../api"; +import { type Song, streamUrl } from "../api"; +import { downloadUrl, saveFrom } from "../lib/download"; import { usePlayer } from "../player/player"; import { useI18n } from "../state/i18n"; import { mayCorrectTracks, useLibraryScope } from "../state/library-scope"; @@ -157,24 +159,43 @@ export function useTrackMenu() { onTouchCancel: cancelPress, }); + // A download that could not start says so: the menu that offered it is + // gone, and a click that does nothing reads as a click that was missed. + const [downloadFailed, setDownloadFailed] = useState(false); + useEffect(() => { + if (!downloadFailed) return; + const timer = window.setTimeout(() => setDownloadFailed(false), 6000); + return () => window.clearTimeout(timer); + }, [downloadFailed]); + function close() { const returnTo = opened?.returnTo; setOpened(null); returnTo?.focus(); } + // The menu and the alert are fixed to the viewport, and `fixed` is only that + // when no ancestor makes itself their containing block: a page block rising + // into place carries a transform while it moves, and a fixed child of it is + // placed inside the block instead — the alert landed under the page header. + // A portal to the body takes them out of every ancestor that could. The + // properties dialog needs none: `showModal` puts it in the top layer. const menu = ( <> - {opened ? ( - { - setProperties({ song, returnTo: opened.returnTo }); - setOpened(null); - }} - /> - ) : null} + {opened + ? createPortal( + setDownloadFailed(true)} + onProperties={(song) => { + setProperties({ song, returnTo: opened.returnTo }); + setOpened(null); + }} + />, + document.body, + ) + : null} {properties ? ( ) : null} + {downloadFailed + ? createPortal( + setDownloadFailed(false)} />, + document.body, + ) + : null} ); return { rowProps, menu }; } +/** Fixed above the player, where it is seen whatever the table's length. */ +function DownloadFailed({ onDismiss }: { onDismiss: () => void }) { + const { t } = useI18n(); + return ( +
+ {t("trackMenu.downloadFailed")} + +
+ ); +} + function TrackMenu({ opened, onClose, onProperties, + onDownloadFailed, }: { opened: Opened; onClose: () => void; onProperties: (song: Song) => void; + onDownloadFailed: () => void; }) { const { t } = useI18n(); const player = usePlayer(); @@ -300,6 +347,17 @@ function TrackMenu({ label: t("trackMenu.queue"), run: () => player.enqueue([song]), }, + { + key: "download", + icon: "download", + label: t("trackMenu.download"), + // The original, through a ticket: the browser cannot send the + // session on a navigation, and the ticket re-checks access. + run: () => + void streamUrl(song.id) + .then(({ url }) => saveFrom(downloadUrl(url))) + .catch(onDownloadFailed), + }, { key: "properties", icon: "info", diff --git a/webapp/src/lib/download.test.ts b/webapp/src/lib/download.test.ts new file mode 100644 index 00000000..120e0315 --- /dev/null +++ b/webapp/src/lib/download.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; + +import { downloadUrl } from "./download"; + +describe("a download's URL", () => { + it("asks the ticket for the file", () => { + expect(downloadUrl("/api/v2/stream/abc")).toBe( + "/api/v2/stream/abc?download=1", + ); + }); + + it("keeps a query the ticket URL already carries", () => { + expect(downloadUrl("/api/v2/stream/abc?x=1")).toBe( + "/api/v2/stream/abc?x=1&download=1", + ); + }); +}); diff --git a/webapp/src/lib/download.ts b/webapp/src/lib/download.ts new file mode 100644 index 00000000..92b4dbf3 --- /dev/null +++ b/webapp/src/lib/download.ts @@ -0,0 +1,23 @@ +/** + * Save a URL the browser can fetch without a header, as a file. + * + * A navigation cannot carry the session's `Authorization` header, which is why + * downloads, like playback, go through a stream ticket. The server names the + * file in `Content-Disposition`; `download` without a value leaves that name + * alone, and asks the browser to save rather than open whatever it receives. + */ +export function saveFrom(url: string): void { + const link = document.createElement("a"); + link.href = url; + link.download = ""; + link.rel = "noopener"; + link.style.display = "none"; + document.body.append(link); + link.click(); + link.remove(); +} + +/** The download form of a stream ticket's URL (RFC-012 decision 5). */ +export function downloadUrl(ticketUrl: string): string { + return `${ticketUrl}${ticketUrl.includes("?") ? "&" : "?"}download=1`; +} diff --git a/webapp/src/state/i18n.tsx b/webapp/src/state/i18n.tsx index 8bf8899c..79b0f6d8 100644 --- a/webapp/src/state/i18n.tsx +++ b/webapp/src/state/i18n.tsx @@ -116,6 +116,9 @@ const en = { "trackMenu.label": "Actions for {title}", "trackMenu.play": "Play", "trackMenu.queue": "Add to queue", + "trackMenu.download": "Download", + "trackMenu.downloadFailed": "The download could not start. Try again.", + "trackMenu.dismiss": "Dismiss", "trackMenu.properties": "Properties", "trackMenu.edit": "Edit tags", "properties.kicker": "Properties", @@ -724,6 +727,10 @@ const fr: Record = { "trackMenu.label": "Actions pour {title}", "trackMenu.play": "Lire", "trackMenu.queue": "Ajouter à la file", + "trackMenu.download": "Télécharger", + "trackMenu.downloadFailed": + "Le téléchargement n’a pas pu démarrer. Réessayez.", + "trackMenu.dismiss": "Fermer", "trackMenu.properties": "Propriétés", "trackMenu.edit": "Corriger les tags", "properties.kicker": "Propriétés", diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index f1d721a4..5d2f3c03 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -2803,3 +2803,35 @@ button.danger { gap: var(--space-1); } } + +/* A download that could not start, said where it is seen: above the player, + whatever the length of the table the menu was opened from. */ +.track-menu-alert { + position: fixed; + z-index: 40; + bottom: 7.5rem; + left: 50%; + display: flex; + align-items: center; + gap: var(--space-3); + max-width: min(32rem, calc(100vw - 2rem)); + padding: var(--space-2) var(--space-2) var(--space-2) var(--space-4); + border: 1px solid var(--line-strong); + border-radius: 0.75rem; + background: var(--panel-strong); + color: var(--danger); + box-shadow: var(--shadow); + font-size: var(--text-sm); + translate: -50% 0; +} + +.track-menu-alert-dismiss { + display: grid; + width: 2rem; + min-height: 2rem; + padding: 0; + place-items: center; + border: 0; + background: transparent; + color: var(--text); +} From 38765c93ba21c1960b908d27241d30fa9497ab11 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 07:14:17 +0200 Subject: [PATCH 2/2] fix: close the track menu on any wheel, and read MANAGED_DIR where it lives - The wheel closes the menu over the menu too: the menu does not scroll, so a wheel over it scrolled the page beneath and left it off its row. The e2e test dispatches a wheel on a menu entry and fails when the wheel is only handled outside the menu. - services/mod.rs no longer re-exports MANAGED_DIR, which its children do not share: the uploads module is pub(crate) and media.rs reads the constant from it. Signed-off-by: InstaZDLL --- src/media.rs | 2 +- src/services/mod.rs | 4 +--- webapp/e2e/studio-nocturne.spec.ts | 11 +++++++++++ webapp/src/components/track-menu.tsx | 8 ++++++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/media.rs b/src/media.rs index c67e3e94..2bc16228 100644 --- a/src/media.rs +++ b/src/media.rs @@ -1279,7 +1279,7 @@ async fn download_name(state: &AppState, user_id: Uuid, track: &StreamTrack) -> .relative_path .split('/') .next() - .is_some_and(|first| first == crate::services::MANAGED_DIR); + .is_some_and(|first| first == crate::services::uploads::MANAGED_DIR); if !received { return basename; } diff --git a/src/services/mod.rs b/src/services/mod.rs index 97714e75..a05497e8 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -1374,9 +1374,7 @@ mod shares; mod songs; mod sync; mod track_metadata; -mod uploads; -/// Where received files land in a library, for the download route's naming. -pub(crate) use uploads::MANAGED_DIR; +pub(crate) mod uploads; /// Crate-visible only: the adapter builds the browser-less journey's address /// from it, and nothing outside this binary has a use for it. diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 380b7bd7..febe65bc 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -1471,6 +1471,17 @@ test("opens a track's properties from its context menu", async ({ page }) => { await expect(menu).toBeVisible(); await page.mouse.click(5, 5); await expect(menu).toHaveCount(0); + + // The wheel closes it even turned over the menu, which does not scroll: + // the page beneath would move and leave the menu off its row. Dispatched + // on the entry itself, because a real wheel in a headless browser brings + // side effects of its own that close the menu either way. + await row.click({ button: "right" }); + await expect(menu).toBeVisible(); + await menu + .getByRole("menuitem", { name: "Play" }) + .dispatchEvent("wheel", { deltaY: 120, bubbles: true }); + await expect(menu).toHaveCount(0); }); /** diff --git a/webapp/src/components/track-menu.tsx b/webapp/src/components/track-menu.tsx index 7fd6a200..09b822f9 100644 --- a/webapp/src/components/track-menu.tsx +++ b/webapp/src/components/track-menu.tsx @@ -296,6 +296,10 @@ function TrackMenu({ const away = (event: Event) => { if (!list.current?.contains(event.target as Node)) closeRef.current(); }; + // The wheel closes it wherever it turns, over the menu too: the menu does + // not scroll, so a wheel over it scrolls the page beneath and leaves the + // menu where its row used to be. + const wheel = () => closeRef.current(); const leave = () => closeRef.current(); // A long press opens the menu with the finger still down, and that finger // drifts as it lifts: its moves are the gesture that opened the menu, not @@ -308,7 +312,7 @@ function TrackMenu({ if (armed) away(event); }; document.addEventListener("pointerdown", away, true); - document.addEventListener("wheel", away, { capture: true, passive: true }); + document.addEventListener("wheel", wheel, { capture: true, passive: true }); document.addEventListener("touchstart", arm, { capture: true, passive: true, @@ -321,7 +325,7 @@ function TrackMenu({ window.addEventListener("resize", leave); return () => { document.removeEventListener("pointerdown", away, true); - document.removeEventListener("wheel", away, true); + document.removeEventListener("wheel", wheel, true); document.removeEventListener("touchstart", arm, true); document.removeEventListener("touchmove", dragged, true); window.removeEventListener("blur", leave);