paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(previews): preserve alpha and animation in the preview tier (#1176)

Browse Source 
* fix(gallery): stop the lightbox loading originals to display a photo (#1166)

Stable twin of #1169.

The lightbox read preview_url, which the server only emits once an admin has
flipped lightbox_preview_enabled — off by default. So a stock install fell
straight through to url, the untouched original: a reporter measured 16.5 MB
for a photo whose preview is 345 KB. The lightbox renders its neighbours too,
so opening one photo pulled three originals.

slideshow_url is the same /preview/:id URL, watermark query included, and has
been emitted unconditionally for images since #1015. Preferring it fixes every
existing install with no migration and no admin action.

Two other surfaces bypass PhotoLightbox entirely and had the same bug:

- premium galleries build their own slides with `src: photo.url`. Fixing that
  also required carrying the photo id on the slide, because the download
  handler recovered the photo by matching slide.src against photo.url — a
  derivative src would have made Download a silent no-op.
- the Story layout rendered the full original as its GRID TILE, at
  object-cover in a small card, and its hero rendered one as a full-bleed
  background when hero_url exists for exactly that. Cards now use the preview
  tier (not the thumbnail: thumbnail_fit is seeded to 'cover', so a thumbnail
  would be cropped a second time and reframe every photo) and only load once
  within 200px of the viewport, since every card mounts at page load.

GIF, APNG and PNG keep the original: generatePreviewImage encodes JPEG, which
has neither a second frame nor an alpha channel. The backend fix that removes
this list is the next commit in this stack.

Divergence from the main twin: no responsive `?w=` tiers. #1095 is main-only,
so `lightboxImageUrl` here selects a URL and nothing more. It lives in
`imageTiers.ts` under the same path main uses, so that backporting #1095 later
merges into this file rather than landing beside it.

Verified on this branch: 8 new tests; frontend suite 21 files / 113 tests,
tsc clean.

* fix(gallery): make the Story hero fix actually work on external galleries (#1166)

External review. Same two fixes as the main twin.

hero_url was inert for external media. ensureHeroImage only ever called
resolvePhotoStorageKey, which returns null for external/reference photos by
design — and that null was handed straight to withLocalCopy, which throws, so
the hero route caught it and redirected to the full ORIGINAL. #1078 fixed
exactly this shape for ensurePreviewImage and nobody carried it across. It
stayed invisible until this PR pointed the Story hero at hero_url: on a
managed gallery that is a real saving, on a reference-mode gallery it quietly
changed nothing.

Needed one extra piece here that main already had: generateHeroImage on this
branch ignores outputBasename and always derives the key from the source
basename, so two events referencing the same NAS filename would clobber each
other's hero. It now honours the option, matching generateThumbnail and
generatePreviewImage.

The format bypass trusted mime_type, which is not trustworthy: migration 039
backfilled every pre-existing photo to image/jpeg regardless of what it was,
and adminExternalMedia inserts rows with no mime_type at all — so a
mislabelled PNG sailed past the guard and came back flattened. It now checks
the filename extension as well.

* fix(previews): preserve alpha and animation in the preview tier

Stable twin of #1171. Stacked on the #1166 twin, whose format bypass this
removes.

generatePreviewImage encoded JPEG unconditionally. JPEG has no alpha channel
and no second frame, so a transparent PNG came back flattened onto a solid
background and an animated GIF came back as its first frame — for every
consumer of this tier, not just the lightbox. It was only invisible by default
because the lightbox served originals.

Sources with alpha, or more than one page, are now encoded as WebP, which
carries both and is still far smaller than the original. Ordinary photos stay
JPEG.

- the output extension matches what was written. A PNG source previously
  produced `preview_foo.png` holding JPEG bytes; harmless while the route
  hard-coded image/jpeg, wrong once the encoding varies. Existing keys keep
  working — they are still JPEG and still served as such.
- the preview route derives Content-Type from the key. With nosniff set,
  mislabelling would show a broken image rather than being silently corrected.
  The watermark branch re-encodes to JPEG and now says so.

The frontend guess-by-MIME goes away entirely, including the case it could
never get right: a still and an animated WebP declare the same type.

Divergence from the main twin: no width-tier case. The responsive `?w=`
renditions (#1095) are main-only, so this branch has a single canonical
preview per photo.

Verified on this branch: 5 new backend tests against real Sharp output;
frontend 21 files / 114 tests; full backend suite leaves the same 5
pre-existing failures as origin/stable.

* fix(previews): retire the legacy preview keys, and stop mislabelling watermarked ones

External review. Same two defects as the main twin.

Legacy keys collide with the new naming. The old generator kept the SOURCE
basename verbatim while always writing JPEG, so a `.webp` upload produced
`previews/preview_shot.webp` holding a JPEG. The claim that pre-existing keys
have no .webp suffix was simply wrong. The route now derives Content-Type from
the key and the response carries nosniff, so every photo uploaded as WebP
would have rendered as a broken image in the lightbox. Legacy `.png` keys are
wrong the other way: flattened JPEGs of what may have been transparent
sources, which isPreviewValid would have let stand forever.

Migration 178 clears photos.preview_path outright — all of it, not just the
suspicious extensions, because a `.jpg` key can equally be a flattened
rendition and nothing in the key says so. Previews regenerate lazily on next
view under the new encoder.

The watermark branch mislabelled its output. applyWatermark PRESERVES the
source format on this branch too (watermarkService.js: png stays png, webp
stays webp), and its input is the preview — so the output already matches the
key the header was derived from. Forcing image/jpeg mislabelled every
watermarked WebP preview, and nosniff means the browser would not correct it.

Numbered 178, not 176: this stack does not carry the external-media
migrations, but that stack takes 176 and 177 on this same branch, and two
files sharing a numeric prefix would be confusing even though both would run.

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-26 09:14:42 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent 75facb4d67
commit 9ffbe2f98f
7 changed files with 361 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/gallery/__tests__/lightboxImageUrl.test.ts
+21 -30
View File
@@ -59,39 +59,30 @@ describe('lightboxImageUrl (#1166)', () => {
})).toBe('/api/gallery/g/preview/47?wm=3');
});
it.each(['image/gif', 'image/apng', 'image/png'])(
'keeps the original for %s, which the preview tier would flatten',
it.each(['image/gif', 'image/apng', 'image/png', 'image/webp', 'image/jpeg'])(
'uses the preview tier for %s — the backend preserves alpha and frames now',
(mime_type) => {
// generatePreviewImage encodes JPEG: no second frame, no alpha channel.
expect(lightboxImageUrl({ ...PHOTO, mime_type })).toBe('/api/gallery/g/photo/47');
// The bypass list this replaces existed because generatePreviewImage
// always encoded JPEG. Previews of alpha or multi-page sources are WebP
// now, so there is nothing left for the frontend to guess at.
expect(lightboxImageUrl({ ...PHOTO, mime_type } as Parameters<typeof lightboxImageUrl>[0]))
.toBe('/api/gallery/g/preview/47');
},
);
it.each(['image/gif', 'image/apng', 'image/png', 'image/webp', 'image/jpeg'])(
'uses the preview tier for %s — the backend preserves alpha and frames now',
(mime_type) => {
// The bypass list this replaces existed because generatePreviewImage
// always encoded JPEG. Previews of alpha or multi-page sources are WebP
// now, so there is nothing left for the frontend to guess at — including
// the filename check that worked around migration 039's mislabelling.
expect(lightboxImageUrl({ ...PHOTO, mime_type } as Parameters<typeof lightboxImageUrl>[0]))
.toBe('/api/gallery/g/preview/47');
},
);
it('catches a PNG that migration 039 mislabelled as image/jpeg', () => {
// 039 backfilled every pre-existing photo's mime_type to image/jpeg, and
// the external-media importer inserts rows with none at all — so trusting
// MIME alone lets exactly the transparent photos through.
expect(lightboxImageUrl({
url: '/api/gallery/g/photo/47',
preview_url: null,
slideshow_url: '/api/gallery/g/preview/47',
mime_type: 'image/jpeg',
filename: 'logo-with-alpha.png',
})).toBe('/api/gallery/g/photo/47');
});
it('catches one with no mime_type at all, as external imports write them', () => {
expect(lightboxImageUrl({
url: '/api/gallery/g/photo/47',
preview_url: null,
slideshow_url: '/api/gallery/g/preview/47',
filename: 'animation.gif',
})).toBe('/api/gallery/g/photo/47');
});
it('still uses the preview tier for ordinary still formats', () => {
for (const mime_type of ['image/jpeg', 'image/webp', undefined]) {
expect(lightboxImageUrl({ ...PHOTO, mime_type })).toBe('/api/gallery/g/preview/47');
}
it('ignores a filename that used to force the original', () => {
expect(lightboxImageUrl({ ...PHOTO, filename: 'legacy.png' } as Parameters<typeof lightboxImageUrl>[0]))
.toBe('/api/gallery/g/preview/47');
});
});
frontend/src/components/gallery/imageTiers.ts
+9 -28
View File
@@ -31,34 +31,15 @@ export function lightboxImageUrl(photo: {
url: string;
preview_url?: string | null;
slideshow_url?: string | null;
mime_type?: string;
filename?: string;
original_filename?: string | null;
}): string {
// Animated and transparent formats keep the original. generatePreviewImage
// encodes JPEG, which has neither a second frame nor an alpha channel, so
// routing these through the preview tier would replace an animation with its
// first frame and flatten transparency onto a solid background — a
// regression the toggle-off default never had.
//
// PNG is in the list because that is where transparency is the norm, and
// because an APNG is normally reported as image/png rather than image/apng.
// Animated or alpha WebP declares image/webp exactly like an ordinary still
// and cannot be told apart from MIME.
//
// The proper fix is backend-side, encoding WebP for alpha or multi-page
// sources; when that lands this list goes away entirely.
// Checked against the FILENAME as well as the MIME, because mime_type is not
// trustworthy here: migration 039 backfilled every pre-existing photo as
// image/jpeg regardless of what it was, and the external-media importer
// inserts rows without a mime_type at all. A mislabelled PNG would otherwise
// sail past this and come back flattened.
const ORIGINAL_ONLY = ['image/gif', 'image/apng', 'image/png'];
const ORIGINAL_ONLY_EXT = /\.(gif|apng|png)$/i;
const name = photo.original_filename || photo.filename || '';
if ((photo.mime_type && ORIGINAL_ONLY.includes(photo.mime_type)) || ORIGINAL_ONLY_EXT.test(name)) {
return photo.url;
}
// No format is excluded any more. This used to bypass the preview tier for
// GIF, APNG and PNG because generatePreviewImage always encoded JPEG, which
// has neither an alpha channel nor a second frame — so a transparent source
// came back flattened and an animated one came back as a still. That is
// fixed at the source: previews of alpha or multi-page images are now WebP,
// which carries both, and the guess-by-MIME this file could never make
// correctly (a still and an animated WebP declare the same type) is gone
// with it — including the filename fallback the previous commit needed
// because migration 039 made mime_type untrustworthy.
return photo.preview_url || photo.slideshow_url || photo.url;
}