Skip to content

fix(theming): fix broken custom images introduced by #58224#60198

Merged
miaulalala merged 4 commits intomasterfrom
fix/noid/theming-broken-images-32-0-9
May 8, 2026
Merged

fix(theming): fix broken custom images introduced by #58224#60198
miaulalala merged 4 commits intomasterfrom
fix/noid/theming-broken-images-32-0-9

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented May 6, 2026 •
edited
Loading

Summary

Fixes #60146 — custom theming images (logo, background, favicon, header logo) stopped displaying after upgrading to 32.0.9.

Root cause (three-part bug chain introduced by #58224)

  1. Wrong conversion direction: ImageManager::getImage() attempted to convert raster images (PNG/JPEG) to SVG using Imagick, which cannot do this meaningfully and produces broken output.
  2. Wrong MIME type in response: ThemingController::getImage() called getMimeType() on the stored file, but stored files have no extension, so Imagick/fileinfo returns application/octet-stream. The Content-Type header was therefore wrong, causing browsers to reject the image.
  3. Stale cache files: .svg cache files persisted after an image was replaced, so subsequent requests served the wrong format.

Why disabling libmagickcore-6.q16-6-extra appeared to fix it

Without SVG support, canConvert('SVG') returns false, skipping the broken conversion path entirely and falling back to returning the original file with its correct MIME type.

Fixes

  • Restrict Imagick conversion to SVG→PNG only (the only direction that makes sense)
  • Read the stored MIME type from IAppConfig for extensionless files in ThemingController::getImage()
  • Delete .svg cache files in ImageManager::delete()
  • Inject IAppConfig into ImageManager and read the cachebuster via IAppConfig::getAppValueInt() — IConfig::getAppValue() can return a stale cached value within the same request, causing the URL returned after upload to carry the old buster value
  • Fix FileInputField.vue thumbnail not refreshing on second consecutive upload of the same MIME type: the background computed property only reacted to mime.value, so uploading PNG→PNG left the thumbnail stale. A reactive cacheKey ref now increments on every successful upload.

Test plan

  • Upload a PNG logo → logo displays correctly
  • Upload an SVG logo → logo displays correctly
  • Upload PNG logo, then upload another PNG logo → thumbnail updates both times without page reload
  • Upload SVG logo, then upload PNG logo → thumbnail updates
  • Remove custom logo → reverts to default
  • Verify on an instance with libmagickcore-6.q16-6-extra installed (SVG support enabled)
  • Verify on an instance without SVG support in Imagick

🤖 Generated with Claude Code

@miaulalala miaulalala requested a review from a team as a code owner May 6, 2026 21:35
@miaulalala miaulalala requested review from leftybournes, nfebe, provokateurin and sorbaugh and removed request for a team May 6, 2026 21:35
@miaulalala miaulalala force-pushed the fix/noid/theming-broken-images-32-0-9 branch from d12c01c to 036d52a Compare May 6, 2026 21:43
@miaulalala miaulalala added this to the Nextcloud 34 milestone May 6, 2026
@miaulalala miaulalala force-pushed the fix/noid/theming-broken-images-32-0-9 branch 2 times, most recently from 7638aca to 7954c0e Compare May 7, 2026 08:35
@julien-nc
Copy link
Copy Markdown
Member

julien-nc commented May 7, 2026

Nice! SVGs now work just fine.

There's one small problem. The image thumbnail next to the button is only updated the first time an image is uploaded.

Sequence:

  • reload the page
  • upload an image: the thumbnail is updated
  • upload another image: the thumbnail is not updated (but the image upload is effective on the theme)
  • reload the page
  • upload an image: the thumbnail is updated
  • etc...
Kooha-2026-05-07-10-30-54.webm

@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

Nice! SVGs now work just fine.

There's one small problem. The image thumbnail next to the button is only updated the first time an image is uploaded.

Sequence:

* reload the page

* upload an image: the thumbnail is updated

* upload another image: the thumbnail is not updated (but the image upload is effective on the theme)

* reload the page

* upload an image: the thumbnail is updated

* etc...

Kooha-2026-05-07-10-30-54.webm

Addressed - can you test again?

@julien-nc
Copy link
Copy Markdown
Member

julien-nc commented May 7, 2026

Thanks! It is slightly different now. The thumbnail is only updated if no image was set before.
Now, when an image is set, it does not matter if i just reloaded the page, uploading a new image does not update the thumbnail.

Oh wait (like models say), if an SVG image is currently set and I upload a PNG or a JPEG, the thumbnail updates. It seems like if the type of the new image is different than the current one, it works. But if uploading something of the same type of what's currently set, nope.

@miaulalala miaulalala requested a review from a team as a code owner May 7, 2026 09:44
@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

Thanks! It is slightly different now. The thumbnail is only updated if no image was set before. Now, when an image is set, it does not matter if i just reloaded the page, uploading a new image does not update the thumbnail.

Oh wait (like models say), if an SVG image is currently set and I upload a PNG or a JPEG, the thumbnail updates. It seems like if the type of the new image is different than the current one, it works. But if uploading something of the same type of what's currently set, nope.

cache buster issue. should be fixed now.

@julien-nc
Copy link
Copy Markdown
Member

julien-nc commented May 7, 2026

All good! 🚀 💥

@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

/backport to stable33

@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

/backport to stable32

@miaulalala miaulalala force-pushed the fix/noid/theming-broken-images-32-0-9 branch from a638734 to b4f19b6 Compare May 7, 2026 10:09
@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

/compile amend

@nextcloud-command nextcloud-command force-pushed the fix/noid/theming-broken-images-32-0-9 branch from b4f19b6 to 5a1da4b Compare May 7, 2026 10:46
@miaulalala miaulalala force-pushed the fix/noid/theming-broken-images-32-0-9 branch 2 times, most recently from 4d5bdaa to c1c23eb Compare May 7, 2026 11:55
@miaulalala
Copy link
Copy Markdown
Contributor Author

miaulalala commented May 7, 2026

/compile amend

@miaulalala miaulalala enabled auto-merge May 7, 2026 12:30
@nextcloud-command nextcloud-command force-pushed the fix/noid/theming-broken-images-32-0-9 branch from c1c23eb to fb543b8 Compare May 7, 2026 13:49
miaulalala added 4 commits May 7, 2026 19:40
@miaulalala @susnux
fix(theming): fix broken custom images introduced in 32.0.9
8a0080c 
PR #58224 introduced a raster→SVG conversion path in ImageManager::getImage()
that breaks display of custom theming images. The root cause is a three-part
bug chain:

1. getImage() attempted to convert raster images (PNG/JPEG) to SVG format,
   which Imagick cannot do meaningfully and produces broken output.
2. getMimeType() returns 'application/octet-stream' for extensionless stored
   files, so the Content-Type response header was wrong.
3. Stale .svg cache files persisted after image replacement, causing
   subsequent requests to serve the wrong format.

Fix by:
- Restricting the Imagick conversion to SVG→PNG only (not raster→SVG)
- Reading the stored MIME type from IAppConfig for extensionless files in
  ThemingController::getImage()
- Deleting .svg cache files in ImageManager::delete()
- Injecting IAppConfig into ImageManager and reading the cachebuster via
  IAppConfig::getAppValueInt() so the URL returned after upload always
  carries the freshly-incremented value (IConfig::getAppValue() can return
  a stale cached value within the same request)
- Updating the FileInputField Vue component to use a reactive cacheKey ref
  that increments on every upload, so the thumbnail refreshes even when the
  MIME type of the new image is the same as the old one

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala @susnux
test(theming): update tests for ImageManager and ThemingController fixes
e54963c 
- ImageManagerTest: inject IAppConfig mock, switch cachebuster assertions
  from IConfig::getAppValue to IAppConfig::getAppValueInt, add
  testGetImageSvgToSvg and testGetImageSvgToPng, update mockGetImage to
  reflect the corrected getImage() logic
- ThemingControllerTest: update getImage and getManifest tests to use
  IAppConfig::getAppValueString for MIME type and cachebuster lookups,
  add testGetLogoOriginalFile for the extensionless-file MIME path

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala @susnux
chore(psalm): update deprecated method baseline
4757b2e 
- Remove stale ThemingController entry (deprecated IConfig::getAppValue
  calls replaced with IAppConfig::getAppValueString)
- Add CommentsEventListener::getEvent() (pre-existing deprecated usage
  not previously baselined)

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala @susnux
fix(reuse): add apps/appstore/l10n to REUSE.toml coverage
6404d75 
New Transifex translation files landed in apps/appstore/l10n/ without
a matching annotation block, causing the REUSE compliance check to fail.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@susnux susnux force-pushed the fix/noid/theming-broken-images-32-0-9 branch from fb543b8 to 6404d75 Compare May 7, 2026 17:40
@miaulalala miaulalala merged commit b002443 into master May 8, 2026
202 of 207 checks passed
@miaulalala miaulalala deleted the fix/noid/theming-broken-images-32-0-9 branch May 8, 2026 10:40
This was referenced May 8, 2026
Draft
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@SebastianKrupinski SebastianKrupinski SebastianKrupinski approved these changes

@nfebe nfebe Awaiting requested review from nfebe nfebe is a code owner automatically assigned from nextcloud/server-backend

@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend

@provokateurin provokateurin Awaiting requested review from provokateurin provokateurin is a code owner automatically assigned from nextcloud/server-backend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner automatically assigned from nextcloud/server-backend

Assignees

No one assigned

Labels

3. to review Waiting for reviews 32-feedback AI assisted feature: theming regression

Projects

None yet

Milestone

Nextcloud 34

Development

Successfully merging this pull request may close these issues.

[Bug]: Theming broken in 32.0.9

5 participants

@miaulalala @julien-nc @susnux @skjnldsv @SebastianKrupinski