feat(theme): add SVG logo support with currentColor theming#1344
feat(theme): add SVG logo support with currentColor theming#1344JohnVillalovos wants to merge 1 commit intodevelopfrom
Conversation
a1f174d to
0be9b2f
Compare
Replace the default raster logo (librebooking.png) with an SVG (librebooking-v5 horizontal lockup) that uses currentColor so it automatically adopts the active CSS theme color. SVG logos are inlined into the page rather than loaded via <img>, which allows currentColor to inherit the --primary CSS variable. Raster logos (PNG, GIF, JPG) continue to work as before via the <img> fallback path. Changes: - Add librebooking.svg as the new default logo; remove librebooking.png - Pages/Page.php: resolve logo file, inline SVG content into LogoSvgContent Smarty variable, support custom-logo.svg uploads - globalheader.tpl, login.tpl, maintenance.tpl: render inline SVG when LogoSvgContent is set, fall back to <img> for raster logos - ManageThemePresenter.php: allow SVG uploads via the theme admin UI - change_theme.tpl: add .svg to accepted file types and help text - librebooking.css: add .logo and .logo-svg rules; header logo sizes itself to 2× the h5 line height via flexbox stretch Assisted-by: Claude Code (claude.ai/code) using claude-sonnet-4-6
0be9b2f to
ab11a20
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for theme-colored SVG logos by inlining SVG markup (using currentColor) while keeping raster logos on the existing <img> path.
Changes:
- Switch default logo from PNG to SVG and inline SVG content into templates when available.
- Extend theme admin logo upload support to allow
.svg. - Add CSS rules to style inline SVG logos and align header sizing.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
Pages/Page.php |
Chooses default/custom logo, reads SVG content, assigns LogoSvgContent for template inlining. |
Presenters/Admin/ManageThemePresenter.php |
Allows .svg as a logo upload type. |
tpl/globalheader.tpl |
Renders inline SVG logo when LogoSvgContent is set; falls back to <img>. |
tpl/login.tpl |
Renders inline SVG logo when LogoSvgContent is set; falls back to <img>. |
tpl/maintenance.tpl |
Renders inline SVG logo when LogoSvgContent is set; falls back to <img>. |
tpl/Admin/Configuration/change_theme.tpl |
Updates accepted logo file types/help text to include .svg. |
Web/css/librebooking.css |
Adds .logo/.logo-svg styling rules for inline SVG rendering. |
Web/img/librebooking.svg |
Adds new default SVG logo asset. |
Web/img/librebooking.png |
Removes old default raster logo asset. |
| .logo { | ||
| color: var(--primary); | ||
| height: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| } |
There was a problem hiding this comment.
The new .logo rule applies to both the inline-SVG wrapper (<span class="logo">) and the raster fallback (<img class="logo">). Properties like display: flex/align-items/height: 100% are intended for a container and make the raster path harder to reason about, potentially changing sizing behavior vs the existing img.logo { max-height: 50px; }. Consider using a dedicated wrapper class for the container (and keep img.logo styling separate), or scope the flex rules so they don’t apply to the <img> element.
| <div class="input-group input-group-sm"> | ||
| <input type="file" {formname key=LOGO_FILE} class="form-control" id="logoFile" | ||
| accept=".png, .gif, .jpg, .jpeg" /> | ||
| accept=".png, .gif, .jpg, .jpeg, .svg" /> |
There was a problem hiding this comment.
The file input accepts .jpeg, but the server-side validator for logos only allows jpg/png/gif/svg (and Page.php only checks for custom-logo.jpg, not custom-logo.jpeg). This makes .jpeg uploads fail despite being selectable. Either add jpeg support end-to-end (validator + lookup) or remove .jpeg from the accept list/help text for logos.
| accept=".png, .gif, .jpg, .jpeg, .svg" /> | |
| accept=".png, .gif, .jpg, .svg" /> |
| $logoSvgContent = null; | ||
| if (str_ends_with($logoUrl, '.svg')) { | ||
| $svgContent = file_get_contents($this->path . 'img/' . $logoUrl); | ||
| if ($svgContent !== false) { | ||
| // Strip the XML declaration so the SVG can be safely inlined into HTML | ||
| $svgContent = preg_replace('/<\?xml[^?]*\?>\s*/i', '', $svgContent); | ||
| $logoSvgContent = $svgContent; | ||
| } | ||
| } | ||
| $this->smarty->assign('LogoSvgContent', $logoSvgContent); |
There was a problem hiding this comment.
LogoSvgContent is populated by reading an arbitrary .svg file (including custom-logo.svg uploaded via the admin UI) and then rendered with nofilter. Stripping only the XML declaration does not make inline SVG safe—SVG can contain scripts/event handlers/foreignObject that become stored XSS when inlined. Please sanitize/whitelist the SVG content before assigning it (or only inline trusted built-in SVGs and keep uploaded SVGs on the <img> path), and adjust the comment that currently implies the output is safe.
| {if $LogoSvgContent} | ||
| <span class="logo" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | ||
| {else} | ||
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="logo"> | ||
| {/if} |
There was a problem hiding this comment.
{$LogoSvgContent nofilter} inlines raw SVG markup into the page. Given LogoSvgContent can come from an uploaded custom-logo.svg, this can enable stored XSS unless the SVG is sanitized/whitelisted server-side before assignment. Please ensure the value is sanitized (or avoid nofilter by keeping uploaded SVGs on the <img> fallback path).
| {if $LogoSvgContent} | |
| <span class="logo" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | |
| {else} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="logo"> | |
| {/if} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="logo"> |
| {if $LogoSvgContent} | ||
| <span class="logo-svg mx-auto d-block w-50" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | ||
| {else} | ||
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> | ||
| {/if} |
There was a problem hiding this comment.
{$LogoSvgContent nofilter} inlines raw SVG markup into the page. Since LogoSvgContent can be sourced from an uploaded custom-logo.svg, this is a stored XSS vector unless the SVG is sanitized/whitelisted before being assigned in PHP. Please ensure server-side sanitization or keep uploaded SVGs rendered via <img> instead of inline.
| {if $LogoSvgContent} | |
| <span class="logo-svg mx-auto d-block w-50" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | |
| {else} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> | |
| {/if} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> |
| {if $LogoSvgContent} | ||
| <span class="logo-svg mx-auto d-block w-50" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | ||
| {else} | ||
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> | ||
| {/if} |
There was a problem hiding this comment.
{$LogoSvgContent nofilter} inlines raw SVG markup into the page. Because LogoSvgContent can come from an uploaded custom-logo.svg, this becomes stored XSS unless the SVG is sanitized/whitelisted server-side before assignment. Please add SVG sanitization or avoid inlining untrusted uploads.
| {if $LogoSvgContent} | |
| <span class="logo-svg mx-auto d-block w-50" role="img" aria-label="{$Title}">{$LogoSvgContent nofilter}</span> | |
| {else} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> | |
| {/if} | |
| <img src="{$Path}img/{$LogoUrl}?{$Version}" alt="{$Title}" class="mx-auto d-block w-50"> |
| $this->page->RegisterValidator('logoFile', new FileUploadValidator($this->page->GetLogoFile())); | ||
| $this->page->RegisterValidator('logoFileExt', new FileTypeValidator($this->page->GetLogoFile(), ['jpg', 'png', 'gif'])); | ||
| $this->page->RegisterValidator('logoFileExt', new FileTypeValidator($this->page->GetLogoFile(), ['jpg', 'png', 'gif', 'svg'])); | ||
| $this->page->RegisterValidator('cssFile', new FileUploadValidator($this->page->GetCssFile())); |
There was a problem hiding this comment.
Allowing .svg in the logo upload validator, combined with the new inline rendering path, enables stored XSS if a malicious SVG is uploaded (extension-only validation is insufficient). Please add SVG sanitization/whitelisting during upload (or before inlining), or disallow inline rendering for uploaded SVGs.
Replace the default raster logo (librebooking.png) with an SVG (librebooking-v5 horizontal lockup) that uses currentColor so it automatically adopts the active CSS theme color.
SVG logos are inlined into the page rather than loaded via
, which allows currentColor to inherit the --primary CSS variable. Raster logos (PNG, GIF, JPG) continue to work as before via the
fallback path.
Changes:
Assisted-by: Claude Code (claude.ai/code) using claude-sonnet-4-6