fix: correct HEIC/HEIF/AVIF preview URL for local/FTP files#759
Open
guideuhome wants to merge 1 commit into
Open
fix: correct HEIC/HEIF/AVIF preview URL for local/FTP files#759guideuhome wants to merge 1 commit into
guideuhome wants to merge 1 commit into
Conversation
- Use currentUrl from model instead of fileAttribute.getName() for file/ftp URLs. After super.filePreviewHandle() downloads the file, the actual path is stored as currentUrl in the model. Using just the filename causes the heic.ftl template to construct incorrect image URLs. - Add avif suffix to HEIC template routing. The wasm_heif.js decoder in static/heic/src/worker.js already supports AVIF decoding, and FileType.java already lists avif in PICTURE_TYPES. This ensures AVIF files also benefit from client-side conversion. Closes kekingcn#664
Open
klboke
requested changes
Jun 24, 2026
klboke
left a comment
Contributor
There was a problem hiding this comment.
@guideuhome thanks for the fix. The currentUrl change for file/ftp HEIC/HEIF previews looks reasonable, but I do not think this PR is safe to merge as-is because of the AVIF routing change.
Blocking issue:
PictureFilePreviewImpl.java now routes avif to HEIC_FILE_PREVIEW_PAGE, but the current HEIC worker cannot actually decode AVIF in this repo state.
Details:
server/src/main/resources/static/heic/src/worker.jsonly imports/heic/src/wasm_heif.js.- The AVIF branch later references
wasm_avif, which is not loaded, so AVIF preview will fail withReferenceError. - The repository has
wasm_avif.js, but does not include the requiredwasm_avif.wasm;wasm_avif.jsattempts to loadwasm_avif.wasmat runtime.
This means AVIF files may regress from browser-native preview to a broken HEIC template path.
Suggested fix:
- If this PR is mainly for issue #664 HEIC/HEIF file/ftp URL handling, remove the
avifrouting change and keep only thecurrentUrlfix. - If AVIF support is intended here, please also load
wasm_avif.js, add the matchingwasm_avif.wasmasset, and add at least a small fixture/test or static resource assertion covering AVIF.
Validation I ran locally:
mvn -q -pl server -DskipTests compilepasses on the PR branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two issues in HEIC/HEIF/AVIF file preview implementation:
Bug Fix: Wrong URL for local/FTP files
When previewing HEIC/HEIF/AVIF files from
file://orftp://URLs,PictureFilePreviewImplwas settingimgUrlstofileAttribute.getName()(just the filename). However,super.filePreviewHandle()downloads the file and stores the actual relative path inmodel.currentUrl. Theheic.ftltemplate usesimgUrlsto construct image URLs, so a bare filename would produce incorrect URLs like${baseUrl}photo.heicinstead of the correct downloaded file path.Fix: Use
model.getAttribute("currentUrl")— the actual downloaded file path — with a fallback tofileAttribute.getName()ifcurrentUrlis null.Enhancement: Add AVIF suffix routing
The
wasm_heif.jsdecoder instatic/heic/src/worker.jsalready supports AVIF decoding (it detects AVIF by checkingarray[8] == 0x61).FileType.javaalso already listsavifinPICTURE_TYPES. However, the suffix check inPictureFilePreviewImpldidn't route AVIF to the HEIC template, causing AVIF files to use the generic picture template without client-side conversion support.Fix: Add
suffix.equalsIgnoreCase("avif")to the HEIC template routing condition.Testing
Closes #664