From 399001003ea98eb26c654628fc082e683986b487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Fern=C3=A1ndez?= Date: Wed, 16 Aug 2023 13:33:39 +0200 Subject: [PATCH] refactor: address review comments about downloads Given initiating a download is browser-specific and we already have the "Copy stream URL" option, we can just leave it alone and let the user fire downloads manually to avoid extra browser shenanigans and checks --- frontend/locales/en-US.json | 2 + .../src/components/Dialogs/ConfirmDialog.vue | 21 ++- frontend/src/components/Item/ItemMenu.vue | 157 +++++++----------- .../src/components/Playback/TrackList.vue | 6 +- frontend/src/pages/library/_itemId/index.vue | 9 +- frontend/src/store/userLibraries.ts | 11 -- frontend/src/utils/browser-detection.ts | 26 +-- frontend/src/utils/file-download.ts | 88 ---------- frontend/src/utils/items.ts | 106 +++++------- 9 files changed, 134 insertions(+), 292 deletions(-) delete mode 100644 frontend/src/utils/file-download.ts diff --git a/frontend/locales/en-US.json b/frontend/locales/en-US.json index 80a5af95434..e255140fdd6 100644 --- a/frontend/locales/en-US.json +++ b/frontend/locales/en-US.json @@ -2,6 +2,7 @@ "3DFormat": "3D format", "NoMediaSourcesAvailable": "No media sources available", "NoMediaStreamsAvailable": "No media streams available for the selected source", + "accept": "Accept", "actor": "Actor", "actors": "Actors", "addNewPerson": "Add a new person", @@ -46,6 +47,7 @@ "connect": "Connect", "continueListening": "Continue listening", "continueWatching": "Continue watching", + "copyPrompt": "Copy the following text into the clipboard?", "copyStreamURL": "Copy Stream URL", "criticRating": "Critic rating", "customRating": "Custom rating", diff --git a/frontend/src/components/Dialogs/ConfirmDialog.vue b/frontend/src/components/Dialogs/ConfirmDialog.vue index 6fb015c17a6..2c3238d138a 100644 --- a/frontend/src/components/Dialogs/ConfirmDialog.vue +++ b/frontend/src/components/Dialogs/ConfirmDialog.vue @@ -9,10 +9,11 @@ - - - {{ state.text }} - + + + {{ t('cancel') }} @@ -31,12 +32,14 @@ diff --git a/frontend/src/components/Item/ItemMenu.vue b/frontend/src/components/Item/ItemMenu.vue index f4b2bc89aa8..727f0ec0b90 100644 --- a/frontend/src/components/Item/ItemMenu.vue +++ b/frontend/src/components/Item/ItemMenu.vue @@ -64,8 +64,6 @@ import IMdiCloudSearch from 'virtual:icons/mdi/cloud-search-outline'; import IMdiContentCopy from 'virtual:icons/mdi/content-copy'; import IMdiDelete from 'virtual:icons/mdi/delete'; import IMdiDisc from 'virtual:icons/mdi/disc'; -import IMdiDownload from 'virtual:icons/mdi/download'; -import IMdiDownloadMultiple from 'virtual:icons/mdi/download-multiple'; import IMdiInformation from 'virtual:icons/mdi/information'; import IMdiPlaylistMinus from 'virtual:icons/mdi/playlist-minus'; import IMdiPlaylistPlus from 'virtual:icons/mdi/playlist-plus'; @@ -82,16 +80,11 @@ import { canInstantMix, canRefreshMetadata, canResume, - getItemDownloadObject, - getItemSeasonDownloadObjects, - getItemSeriesDownloadObjects + getItemDownloadUrl, + getItemSeasonDownloadMap, + getItemSeriesDownloadMap } from '@/utils/items'; import { playbackManagerStore, taskManagerStore } from '@/store'; -import { - DownloadableFile, - canBrowserDownloadItem, - downloadFiles -} from '@/utils/file-download'; type MenuOption = { title: string; @@ -299,79 +292,61 @@ const identifyItemAction = { identifyItemDialog.value = true; } }; -const sharedDownloadAction = async (): Promise => { - if (menuProps.item.Id && menuProps.item.Type && menuProps.item.Path) { - let downloadURLs: DownloadableFile[] = []; - - switch (menuProps.item.Type) { - case 'Season': { - downloadURLs = await getItemSeasonDownloadObjects(menuProps.item.Id); - break; - } - case 'Series': { - downloadURLs = await getItemSeriesDownloadObjects(menuProps.item.Id); - break; - } - default: { - const url = getItemDownloadObject( - menuProps.item.Id, - menuProps.item.Path - ); - - if (url) { - downloadURLs = [url]; - } - - break; - } - } - - if (downloadURLs) { - try { - await downloadFiles(downloadURLs); - } catch (error) { - console.error(error); - - useSnackbar(errorMessage, 'error'); - } - } else { - console.error( - 'Unable to get download URL for selected item/series/season' - ); - useSnackbar(errorMessage, 'error'); - } - } -}; -const singleDownloadAction = { - title: t('downloadItem', 1), - icon: IMdiDownload, - action: sharedDownloadAction -}; -const multiDownloadAction = { - title: t('downloadItem', 2), - icon: IMdiDownloadMultiple, - action: sharedDownloadAction -}; -const copyStreamURLAction = { +const copyDownloadURLAction = { title: t('copyStreamURL'), icon: IMdiContentCopy, action: async (): Promise => { - if (menuProps.item.Id) { - const downloadHref = getItemDownloadObject(menuProps.item.Id); - const clipboard = useClipboard(); + const clipboard = useClipboard(); + let streamUrls: Map | string | undefined; - try { - if (clipboard.isSupported.value) { - if (downloadHref?.url) { - await clipboard.copy(downloadHref.url); - } - } else { - throw new ReferenceError('Unsupported clipboard operation'); + if (!clipboard.isSupported.value) { + useSnackbar(t('clipboardUnsupported'), 'error'); + + return; + } + + if (menuProps.item.Id) { + switch (menuProps.item.Type) { + case 'Season': { + streamUrls = await getItemSeasonDownloadMap(menuProps.item.Id); + break; + } + case 'Series': { + streamUrls = await getItemSeriesDownloadMap(menuProps.item.Id); + break; + } + default: { + streamUrls = getItemDownloadUrl(menuProps.item.Id); + break; } - } catch (error) { - error instanceof ReferenceError - ? useSnackbar(t('clipboardUnsupported'), 'error') - : useSnackbar(errorMessage, 'error'); + } + + /** + * The Map is mapped to an string like: EpisodeName: DownloadUrl + */ + const text = streamUrls + ? typeof streamUrls === 'string' + ? streamUrls + : [...streamUrls.entries()] + .map(([k, v]) => `(${k}) - ${v}`) + .join('\n') + : undefined; + + const copyAction = async (txt: string): Promise => { + await clipboard.copy(txt); + useSnackbar(t('clipboardSuccess'), 'success'); + }; + + if (text) { + await (typeof streamUrls === 'string' + ? copyAction(text) + : useConfirmDialog(async () => await copyAction(text), { + title: t('copyPrompt'), + text: text, + confirmText: t('accept') + })); + } else { + useSnackbar(errorMessage, 'error'); } } } @@ -443,22 +418,15 @@ function getPlaybackOptions(): MenuOption[] { /** * Copy and download action for the current selected item */ -function getCopyDownloadOptions(): MenuOption[] { - const copyDownloadActions: MenuOption[] = []; - - if (menuProps.item.CanDownload) { - copyDownloadActions.push(copyStreamURLAction); - - if (canBrowserDownloadItem(menuProps.item)) { - copyDownloadActions.push(singleDownloadAction); +function getCopyOptions(): MenuOption[] { + const copyActions: MenuOption[] = []; + const remote = useRemote(); - if (['Season', 'Series'].includes(menuProps.item.Type || '')) { - copyDownloadActions.push(multiDownloadAction); - } - } + if (remote.auth.currentUser?.Policy?.EnableContentDownloading) { + copyActions.push(copyDownloadURLAction); } - return copyDownloadActions; + return copyActions; } /** @@ -483,7 +451,10 @@ function getLibraryOptions(): MenuOption[] { } } - if (menuProps.item.CanDelete) { + if ( + remote.auth.currentUser?.Policy?.EnableContentDeletion || + remote.auth.currentUser?.Policy?.EnableContentDeletionFromFolders + ) { libraryOptions.push(deleteItemAction); } @@ -494,7 +465,7 @@ const options = computed(() => { return [ getQueueOptions(), getPlaybackOptions(), - getCopyDownloadOptions(), + getCopyOptions(), getLibraryOptions() ]; }); diff --git a/frontend/src/components/Playback/TrackList.vue b/frontend/src/components/Playback/TrackList.vue index 47d5556f897..dacd3592bd0 100644 --- a/frontend/src/components/Playback/TrackList.vue +++ b/frontend/src/components/Playback/TrackList.vue @@ -113,11 +113,7 @@ async function fetch(): Promise { parentId: props.item.Id, sortBy: ['SortName'], sortOrder: [SortOrder.Ascending], - fields: [ - ItemFields.MediaSources, - ItemFields.CanDelete, - ItemFields.CanDownload - ] + fields: [ItemFields.MediaSources] }) ).data.Items; } diff --git a/frontend/src/pages/library/_itemId/index.vue b/frontend/src/pages/library/_itemId/index.vue index d3ef3d74e25..ead83a5d51a 100644 --- a/frontend/src/pages/library/_itemId/index.vue +++ b/frontend/src/pages/library/_itemId/index.vue @@ -58,7 +58,11 @@ import { computed, onMounted, ref, watch } from 'vue'; import { useI18n } from 'vue-i18n'; import { useRoute } from 'vue-router'; -import { BaseItemDto, BaseItemKind } from '@jellyfin/sdk/lib/generated-client'; +import { + BaseItemDto, + BaseItemKind, + ItemFields +} from '@jellyfin/sdk/lib/generated-client'; import { getItemsApi } from '@jellyfin/sdk/lib/utils/api/items-api'; import { getArtistsApi } from '@jellyfin/sdk/lib/utils/api/artists-api'; import { getPersonsApi } from '@jellyfin/sdk/lib/utils/api/persons-api'; @@ -283,7 +287,8 @@ async function refreshItems(): Promise { filters.value.features.includes('HasThemeVideo') || undefined, isHd: filters.value.types.includes('isHD') || undefined, is4K: filters.value.types.includes('is4K') || undefined, - is3D: filters.value.types.includes('is3D') || undefined + is3D: filters.value.types.includes('is3D') || undefined, + fields: Object.values(ItemFields) }) ).data; break; diff --git a/frontend/src/store/userLibraries.ts b/frontend/src/store/userLibraries.ts index 3e5f6fc11aa..46d874ae61a 100644 --- a/frontend/src/store/userLibraries.ts +++ b/frontend/src/store/userLibraries.ts @@ -149,8 +149,6 @@ class UserLibrariesStore { fields: [ ItemFields.PrimaryImageAspectRatio, ItemFields.MediaSources, - ItemFields.CanDelete, - ItemFields.CanDownload, ItemFields.ProviderIds ], imageTypeLimit: 1, @@ -180,8 +178,6 @@ class UserLibrariesStore { fields: [ ItemFields.PrimaryImageAspectRatio, ItemFields.MediaSources, - ItemFields.CanDelete, - ItemFields.CanDownload, ItemFields.ProviderIds ], imageTypeLimit: 1, @@ -211,8 +207,6 @@ class UserLibrariesStore { fields: [ ItemFields.PrimaryImageAspectRatio, ItemFields.MediaSources, - ItemFields.CanDelete, - ItemFields.CanDownload, ItemFields.ProviderIds ], imageTypeLimit: 1, @@ -244,9 +238,6 @@ class UserLibrariesStore { fields: [ ItemFields.PrimaryImageAspectRatio, ItemFields.MediaSources, - - ItemFields.CanDelete, - ItemFields.CanDownload, ItemFields.ProviderIds ], imageTypeLimit: 1, @@ -276,8 +267,6 @@ class UserLibrariesStore { ItemFields.Overview, ItemFields.PrimaryImageAspectRatio, ItemFields.MediaSources, - ItemFields.CanDelete, - ItemFields.CanDownload, ItemFields.ProviderIds ], enableImageTypes: [ImageType.Backdrop, ImageType.Logo], diff --git a/frontend/src/utils/browser-detection.ts b/frontend/src/utils/browser-detection.ts index 3df68d1d3b8..0fa8d01e535 100644 --- a/frontend/src/utils/browser-detection.ts +++ b/frontend/src/utils/browser-detection.ts @@ -28,16 +28,10 @@ export function supportsMediaSource(): boolean { * @private * @static * @param key - Key for which to perform a check. - * @param caseSensitive - Whether the check should be case sensitive. * @returns Determines if user agent of navigator contains a key */ -function userAgentContains(key: string, caseSensitive = true): boolean { - let userAgent = navigator.userAgent || ''; - - if (!caseSensitive) { - key = key.toLowerCase(); - userAgent = userAgent.toLowerCase(); - } +function userAgentContains(key: string): boolean { + const userAgent = navigator.userAgent || ''; return userAgent.includes(key); } @@ -63,22 +57,6 @@ export function isEdge(): boolean { return userAgentContains('Edg/') || userAgentContains('Edge/'); } -/** - * Check if the current platform is Microsoft Edge UWP. - * - * @static - * @returns Determines if browser is Microsoft Edge UWP. - */ -export function isEdgeUWP(): boolean { - if (!isEdge()) { - return false; - } - - return ( - userAgentContains('msapphost', false) || userAgentContains('webview', false) - ); -} - /** * Check if the current platform is Chromium based. * diff --git a/frontend/src/utils/file-download.ts b/frontend/src/utils/file-download.ts deleted file mode 100644 index ea8fe3b52c4..00000000000 --- a/frontend/src/utils/file-download.ts +++ /dev/null @@ -1,88 +0,0 @@ -import { BaseItemDto } from '@jellyfin/sdk/lib/generated-client'; -import { isEdgeUWP, isFirefox, isPs4, isTv, isXbox } from './browser-detection'; - -export interface DownloadableFile { - // The file URL - url: string; - // The filename, including the file extension - fileName: string; -} - -/** - * Check if the url is on the same domain as the current page. - * - * @param url - The url to check. - */ -function sameDomain(url: string): boolean { - const a = document.createElement('a'); - - a.href = url; - - return ( - window.location.hostname === a.hostname && - window.location.protocol === a.protocol - ); -} - -/** - * Use html tag to download a file. - * - * @param file - An object with `url` and `fileName` properties. - */ -function downloadBrowser(file: DownloadableFile): void { - const a = document.createElement('a'); - - a.download = file.fileName; - a.href = file.url; - // firefox doesn't support `a.click()`... - a.dispatchEvent(new MouseEvent('click')); -} - -/** - * Check if the browser are able to download the item. - * - * @param item - The item to check. - */ -export function canBrowserDownloadItem(item: BaseItemDto): boolean { - return ( - !isEdgeUWP() && !isTv() && !isXbox() && !isPs4() && item.Type !== 'Book' - ); -} - -/** - * Download multiple files. - * - * @param filesToDownload - An array of objects with `url` and `fileName` properties. - */ -export async function downloadFiles( - filesToDownload: DownloadableFile | DownloadableFile[] -): Promise { - if (!filesToDownload) { - throw new Error('`filesToDownload` required'); - } - - const files = Array.isArray(filesToDownload) - ? filesToDownload - : [filesToDownload]; - - if (files.length === 0) { - throw new Error( - '`filesToDownload` must be an array with at least one item' - ); - } - - if (document.createElement('a').download === undefined) { - throw new Error('Browser does not support downloading files'); - } - - let delay = 0; - - for (const file of files) { - if (isFirefox() && !sameDomain(file.url)) { - // the download init has to be sequential for firefox if the urls are not on the same domain - setTimeout(downloadBrowser.bind(undefined, file), 100 * ++delay); - } else { - downloadBrowser(file); - } - } -} diff --git a/frontend/src/utils/items.ts b/frontend/src/utils/items.ts index 8c02331ba2f..8b7a8fce68a 100644 --- a/frontend/src/utils/items.ts +++ b/frontend/src/utils/items.ts @@ -9,6 +9,7 @@ import { MediaStream } from '@jellyfin/sdk/lib/generated-client'; import { useRouter } from 'vue-router'; +import { isNil } from 'lodash-es'; import type { RouteNamedMap } from 'vue-router/auto/routes'; import IMdiMovie from 'virtual:icons/mdi/movie'; import IMdiMusic from 'virtual:icons/mdi/music'; @@ -30,7 +31,6 @@ import IMdiAlbum from 'virtual:icons/mdi/album'; import { getItemsApi } from '@jellyfin/sdk/lib/utils/api/items-api'; import { getTvShowsApi } from '@jellyfin/sdk/lib/utils/api/tv-shows-api'; import { ticksToMs } from './time'; -import { DownloadableFile } from './file-download'; import { useRemote } from '@/composables'; /** @@ -473,10 +473,7 @@ export function getMediaStreams( * @param itemPath - The item path. * @returns - A download object. */ -export function getItemDownloadObject( - itemId: string, - itemPath?: string -): DownloadableFile | undefined { +export function getItemDownloadUrl(itemId: string): string | undefined { const remote = useRemote(); const serverAddress = remote.sdk.api?.basePath; @@ -486,83 +483,72 @@ export function getItemDownloadObject( return undefined; } - const fileName = itemPath?.includes('\\') - ? itemPath?.split('\\').pop() - : itemPath?.split('/').pop(); - - return { - url: `${serverAddress}/Items/${itemId}/Download?api_key=${userToken}`, - fileName: fileName ?? '' - }; + return `${serverAddress}/Items/${itemId}/Download?api_key=${userToken}`; } /** - * Get multiple download object for seasons. + * Get a map of an episode name and its download url, given a season. * * @param seasonId - The season ID. - * @returns - An array of download objects. + * @returns - A map: [EpisodeName, DownloadUrl]. */ -export async function getItemSeasonDownloadObjects( +export async function getItemSeasonDownloadMap( seasonId: string -): Promise { +): Promise> { const remote = useRemote(); - - if (remote.sdk.api === undefined) { - return []; + const result = new Map(); + + const episodes = + ( + await remote.sdk.newUserApi(getItemsApi).getItems({ + userId: remote.auth.currentUserId, + parentId: seasonId, + fields: [ItemFields.Overview, ItemFields.CanDownload, ItemFields.Path] + }) + ).data.Items ?? []; + + for (const episode of episodes) { + if (episode.Id && !isNil(episode.Name)) { + const url = getItemDownloadUrl(episode.Id); + + if (url) { + result.set(episode.Name, url); + } + } } - const episodes = ( - await remote.sdk.newUserApi(getItemsApi).getItems({ - userId: remote.auth.currentUserId, - parentId: seasonId, - fields: [ItemFields.Overview, ItemFields.CanDownload, ItemFields.Path] - }) - ).data; - - return ( - episodes.Items?.map((r) => { - if (r.Id && r.Path) { - return getItemDownloadObject(r.Id, r.Path); - } - }).filter( - (r): r is DownloadableFile => - r !== undefined && r.url.length > 0 && r.fileName.length > 0 - ) ?? [] - ); + return result; } /** - * Get download object for a series. - * This will fetch every season for all the episodes. + * Get a map of an episode name and its download url, given a series. * - * @param seriesId - The series ID. - * @returns - An array of download objects. + * @param seasonId - The season ID. + * @returns - A map: [EpisodeName, DownloadUrl]. */ -export async function getItemSeriesDownloadObjects( +export async function getItemSeriesDownloadMap( seriesId: string -): Promise { +): Promise> { const remote = useRemote(); + let result = new Map(); - let mergedStreamURLs: DownloadableFile[] = []; - - if (remote.sdk.api === undefined) { - return []; - } - - const seasons = ( - await remote.sdk.newUserApi(getTvShowsApi).getSeasons({ - userId: remote.auth.currentUserId, - seriesId: seriesId - }) - ).data; + const seasons = + ( + await remote.sdk.newUserApi(getTvShowsApi).getSeasons({ + userId: remote.auth.currentUserId, + seriesId: seriesId + }) + ).data.Items ?? []; - for (const season of seasons.Items || []) { - const seasonURLs = await getItemSeasonDownloadObjects(season.Id || ''); + for (const season of seasons) { + if (season.Id) { + const map = await getItemSeasonDownloadMap(season.Id); - mergedStreamURLs = [...mergedStreamURLs, ...seasonURLs]; + result = new Map([...result, ...map]); + } } - return mergedStreamURLs; + return result; } /**