diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx index 894381d37..5818a5d66 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useState, useRef } from 'react' import { localIsoDate } from '../../utils/localDate' -import { X, Plus, Image, Minus, Check, MapPin, Locate, Camera } from 'lucide-react' +import { X, Plus, Image, Minus, Check, MapPin, Locate, Camera, Play } from 'lucide-react' import { normalizeImageFiles } from '../../utils/convertHeic' import { isVideoFile } from '../../utils/videoPoster' import { type ResilientResult, type UploadProgress } from '../../utils/uploadQueue' @@ -11,7 +11,7 @@ import { getCurrentPositionOnce } from '../../hooks/useGeolocation' import { getApiErrorMessage } from '../../types' import type { JourneyEntry, JourneyPhoto, GalleryPhoto, JourneyTrip } from '../../store/journeyStore' import { MOOD_CONFIG, WEATHER_CONFIG } from '../../pages/journeyDetail/JourneyDetailPage.constants' -import { photoUrl, isValidGeoPoint, geoOnceErrorKey } from '../../pages/journeyDetail/JourneyDetailPage.helpers' +import { photoUrl, posterlessVideo, isValidGeoPoint, geoOnceErrorKey } from '../../pages/journeyDetail/JourneyDetailPage.helpers' import MarkdownToolbar from './MarkdownToolbar' import { DatePicker } from './JourneyDetailPageDatePicker' import CustomTimePicker from '../shared/CustomTimePicker' @@ -527,7 +527,15 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips,
{photos.map((p, idx) => (
1 ? 'ring-2 ring-zinc-900 dark:ring-white ring-offset-1 dark:ring-offset-zinc-900' : ''}`}> - { const img = e.currentTarget; const orig = photoUrl(p, 'original'); if (!img.src.includes('/original')) img.src = orig }} /> + {posterlessVideo(p) ? ( + // No poster to show and falling back to /original would hand + // an the clip itself, so this tile stays a play badge. +
+ +
+ ) : ( + { const img = e.currentTarget; const orig = photoUrl(p, 'original'); if (!img.src.includes('/original')) img.src = orig }} /> + )} {idx === 0 && photos.length > 1 && ( {t('journey.editor.photoFirst')} )} diff --git a/client/src/components/Journey/JourneyDetailPageGalleryView.tsx b/client/src/components/Journey/JourneyDetailPageGalleryView.tsx index 52ba4a41a..e82928427 100644 --- a/client/src/components/Journey/JourneyDetailPageGalleryView.tsx +++ b/client/src/components/Journey/JourneyDetailPageGalleryView.tsx @@ -8,7 +8,7 @@ import { journeyApi, addonsApi, memoriesApi } from '../../api/client' import { useToast } from '../shared/Toast' import { getApiErrorMessage } from '../../types' import type { JourneyEntry, GalleryPhoto, JourneyTrip } from '../../store/journeyStore' -import { photoUrl } from '../../pages/journeyDetail/JourneyDetailPage.helpers' +import { photoUrl, posterlessVideo } from '../../pages/journeyDetail/JourneyDetailPage.helpers' import { ProviderPicker } from './JourneyDetailPageProviderPicker' import { ScrollTrigger } from './JourneyDetailPageScrollTrigger' import EmptyState from '../shared/EmptyState' @@ -151,9 +151,7 @@ export function GalleryView({ entries, gallery, journeyId, userId, trips, onPhot onClick={() => onPhotoClick(allPhotos, i)} onKeyDown={e => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onPhotoClick(allPhotos, i) } }} > - {photo.media_type === 'video' && - photo.provider === 'local' && - !photo.thumbnail_path ? ( + {posterlessVideo(photo) ? ( // Poster-less local video: show a neutral tile.
) : ( diff --git a/client/src/components/Journey/JourneyDetailPagePhotoGrid.test.tsx b/client/src/components/Journey/JourneyDetailPagePhotoGrid.test.tsx new file mode 100644 index 000000000..f893ce6c0 --- /dev/null +++ b/client/src/components/Journey/JourneyDetailPagePhotoGrid.test.tsx @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest' +import { render } from '../../../tests/helpers/render' +import { PhotoImg } from './JourneyDetailPagePhotoGrid' +import type { JourneyPhoto } from '../../store/journeyStore' + +function buildPhoto(overrides: Partial = {}): JourneyPhoto { + return { + id: 1, + entry_id: 10, + photo_id: 515, + sort_order: 0, + shared: 1, + created_at: 0, + provider: 'local', + ...overrides, + } +} + +describe('PhotoImg', () => { + it('asks for the thumbnail of a photo', () => { + const { container } = render() + + expect(container.querySelector('img')).toHaveAttribute('src', '/api/photos/515/thumbnail') + }) + + it('shows the poster of a clip behind the play badge', () => { + const { container } = render() + + expect(container.querySelector('img')).toHaveAttribute('src', '/api/photos/515/thumbnail') + expect(container.querySelector('svg')).not.toBeNull() + }) + + it('draws no image for a clip without a poster, whose thumbnail route answers 404 (#2341)', () => { + // The tile stays black with the play badge on it. An here would only + // add the broken-image glyph the reporter saw. + const { container } = render() + + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('svg')).not.toBeNull() + expect(container.firstElementChild?.className).toContain('bg-black') + }) + + it('still asks the server for a provider clip, which serves its own poster', () => { + const { container } = render() + + expect(container.querySelector('img')).toHaveAttribute('src', '/api/photos/515/thumbnail') + }) +}) diff --git a/client/src/components/Journey/JourneyDetailPagePhotoGrid.tsx b/client/src/components/Journey/JourneyDetailPagePhotoGrid.tsx index 2123c209d..93869d6a7 100644 --- a/client/src/components/Journey/JourneyDetailPagePhotoGrid.tsx +++ b/client/src/components/Journey/JourneyDetailPagePhotoGrid.tsx @@ -1,6 +1,6 @@ import { Image, Play } from 'lucide-react' import type { JourneyPhoto } from '../../store/journeyStore' -import { photoUrl } from '../../pages/journeyDetail/JourneyDetailPage.helpers' +import { photoUrl, posterlessVideo } from '../../pages/journeyDetail/JourneyDetailPage.helpers' export function PhotoImg({ photo, className, style }: { photo: JourneyPhoto; className?: string; style?: React.CSSProperties }) { const src = photoUrl(photo, 'thumbnail') @@ -11,12 +11,16 @@ export function PhotoImg({ photo, className, style }: { photo: JourneyPhoto; cla className={`relative overflow-hidden ${isVideo ? 'bg-black' : ''} ${className || ''}`} style={style} > - + {/* A clip without a poster keeps the black tile and the play badge; asking + for its thumbnail would only draw the broken-image glyph over them. */} + {!posterlessVideo(photo) && ( + + )} {isVideo && (
diff --git a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts index 0242c2ced..9b8bfcf01 100644 --- a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts +++ b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.test.ts @@ -4,6 +4,7 @@ import { distanceBetweenGeoPoints, groupPhotosByDate, matchJourneyEntries, + posterlessVideo, sortProviderPhotos, utcOffsetMinutesForDay, } from './JourneyDetailPage.helpers'; @@ -207,3 +208,20 @@ describe('matchJourneyEntries', () => { expect(matchJourneyEntries(entries, 'reykjavik')).toEqual([]); }); }); + +describe('posterlessVideo (#2341)', () => { + it('is a local clip whose poster frame was never grabbed', () => { + expect(posterlessVideo({ media_type: 'video', provider: 'local', thumbnail_path: null })).toBe(true); + expect(posterlessVideo({ media_type: 'video', provider: 'local' })).toBe(true); + }); + + it('is not a clip that has its poster, whatever the file is called', () => { + expect(posterlessVideo({ media_type: 'video', provider: 'local', thumbnail_path: 'journey/poster.jpg' })).toBe(false); + }); + + it('is never a photo, and never a provider clip, whose poster the provider serves', () => { + expect(posterlessVideo({ media_type: 'image', provider: 'local', thumbnail_path: null })).toBe(false); + expect(posterlessVideo({ provider: 'local', thumbnail_path: null })).toBe(false); + expect(posterlessVideo({ media_type: 'video', provider: 'immich', thumbnail_path: null })).toBe(false); + }); +}); diff --git a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts index c1d6199ab..789410b7a 100644 --- a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts +++ b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts @@ -73,6 +73,21 @@ export function photoUrl(p: { photo_id: number }, size: 'thumbnail' | 'original' return `/api/photos/${p.photo_id}/${size}`; } +/** + * A clip that came up without a poster frame: the browser could not decode it + * at upload time, so nothing was grabbed. Its thumbnail route answers 404 on + * purpose rather than streaming the whole file, and an pointed at it + * shows the broken-image glyph (#2341). A provider clip is not one of these, + * its poster comes from the provider whether or not a local path is recorded. + */ +export function posterlessVideo(p: { + media_type?: string | null; + provider?: string | null; + thumbnail_path?: string | null; +}): boolean { + return p.media_type === 'video' && p.provider === 'local' && !p.thumbnail_path; +} + /** * Which calendar day a provider photo belongs to. * diff --git a/server/src/middleware/globalMiddleware.ts b/server/src/middleware/globalMiddleware.ts index 2b6321d22..4bd5df726 100644 --- a/server/src/middleware/globalMiddleware.ts +++ b/server/src/middleware/globalMiddleware.ts @@ -265,6 +265,13 @@ export function applyGlobalMiddleware( ], workerSrc: ["'self'", "blob:"], childSrc: ["'self'", "blob:"], + // blob: because a picked clip is previewed and its poster frame grabbed + // through a