mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(journey): let the browser read a picked clip, so it previews and gets a poster
A clip picked in the entry editor is previewed through a <video> on an object URL, and its poster frame is grabbed the same way before anything is uploaded. The CSP never named media-src, so the browser fell back to default-src, which does not admit blob: at all. The preview stayed empty and the poster capture failed silently, so every clip landed without a poster and its thumbnail route answered 404 (issue #2341, the part that survived 4.3.0). None of it showed in development, where Vite serves the document and helmet never sees it. media-src now carries 'self' and blob:. For the clips that still come up without a poster, because the browser cannot decode them, the entry card and the editor tile draw the play badge on a black tile instead of asking for a thumbnail that is 404 by design, the way the gallery already did. Closes #2341
This commit is contained in:
@@ -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,
|
||||
<div className="flex flex-wrap gap-2">
|
||||
{photos.map((p, idx) => (
|
||||
<div key={p.id} className={`w-20 h-20 rounded-xl overflow-hidden relative group ${idx === 0 && photos.length > 1 ? 'ring-2 ring-zinc-900 dark:ring-white ring-offset-1 dark:ring-offset-zinc-900' : ''}`}>
|
||||
<img src={photoUrl(p)} className="w-full h-full object-cover" alt="" onError={e => { 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 <img> the clip itself, so this tile stays a play badge.
|
||||
<div className="w-full h-full bg-black flex items-center justify-center text-white">
|
||||
<Play size={18} className="ml-0.5" fill="currentColor" />
|
||||
</div>
|
||||
) : (
|
||||
<img src={photoUrl(p)} className="w-full h-full object-cover" alt="" onError={e => { const img = e.currentTarget; const orig = photoUrl(p, 'original'); if (!img.src.includes('/original')) img.src = orig }} />
|
||||
)}
|
||||
{idx === 0 && photos.length > 1 && (
|
||||
<span className="absolute bottom-0.5 left-0.5 px-1 py-px rounded text-[8px] font-bold bg-zinc-900/70 text-white">{t('journey.editor.photoFirst')}</span>
|
||||
)}
|
||||
|
||||
@@ -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.
|
||||
<div className="w-full h-full bg-zinc-200 dark:bg-zinc-800" />
|
||||
) : (
|
||||
|
||||
@@ -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> = {}): 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(<PhotoImg photo={buildPhoto()} />)
|
||||
|
||||
expect(container.querySelector('img')).toHaveAttribute('src', '/api/photos/515/thumbnail')
|
||||
})
|
||||
|
||||
it('shows the poster of a clip behind the play badge', () => {
|
||||
const { container } = render(<PhotoImg photo={buildPhoto({ media_type: 'video', thumbnail_path: 'journey/poster.jpg' })} />)
|
||||
|
||||
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 <img> here would only
|
||||
// add the broken-image glyph the reporter saw.
|
||||
const { container } = render(<PhotoImg photo={buildPhoto({ media_type: 'video', thumbnail_path: null })} />)
|
||||
|
||||
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(<PhotoImg photo={buildPhoto({ media_type: 'video', provider: 'immich', thumbnail_path: null })} />)
|
||||
|
||||
expect(container.querySelector('img')).toHaveAttribute('src', '/api/photos/515/thumbnail')
|
||||
})
|
||||
})
|
||||
@@ -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}
|
||||
>
|
||||
<img
|
||||
src={src}
|
||||
alt=""
|
||||
className={`w-full h-full ${isVideo ? 'object-contain' : 'object-cover'}`}
|
||||
loading="lazy"
|
||||
/>
|
||||
{/* 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) && (
|
||||
<img
|
||||
src={src}
|
||||
alt=""
|
||||
className={`w-full h-full ${isVideo ? 'object-contain' : 'object-cover'}`}
|
||||
loading="lazy"
|
||||
/>
|
||||
)}
|
||||
|
||||
{isVideo && (
|
||||
<div className="absolute inset-0 flex items-center justify-center pointer-events-none">
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 <img> 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.
|
||||
*
|
||||
|
||||
@@ -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 <video> on an object URL, before any byte reaches the server.
|
||||
// Unset, this fell back to default-src, which refuses blob: outright: the
|
||||
// editor showed nothing and every clip landed without a poster, so its
|
||||
// thumbnail answered 404 (#2341). Invisible in dev, where Vite serves the
|
||||
// document without this header.
|
||||
mediaSrc: ["'self'", "blob:"],
|
||||
fontSrc: ["'self'", "https://fonts.gstatic.com", "data:"],
|
||||
// 'self' so same-origin file previews can embed PDFs via <object>/<embed>
|
||||
// (Firefox/Chrome enforce object-src; 'none' broke inline PDF previews there).
|
||||
|
||||
@@ -103,6 +103,21 @@ describe('global CSP: script-src', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('global CSP: media-src (#2341)', () => {
|
||||
it('allows blob:, which is where a picked clip lives until it is uploaded', async () => {
|
||||
// The editor previews the clip and grabs its poster frame through a <video>
|
||||
// on an object URL. With media-src unset the browser fell back to
|
||||
// default-src, which never admits blob:, so the preview stayed empty and
|
||||
// the clip went up without a poster. Nothing of this shows in dev: Vite
|
||||
// serves the document there and helmet never sees it.
|
||||
expect(await directiveSources('media-src')).toContain('blob:');
|
||||
});
|
||||
|
||||
it("keeps 'self' so the uploaded clip still plays from /api/photos", async () => {
|
||||
expect(await directiveSources('media-src')).toContain("'self'");
|
||||
});
|
||||
});
|
||||
|
||||
describe('forced-HTTPS redirect', () => {
|
||||
const saved = { FORCE_HTTPS: process.env.FORCE_HTTPS, APP_URL: process.env.APP_URL };
|
||||
|
||||
|
||||
Reference in New Issue
Block a user