Skip to content

feat(theme): add SVG logo support with currentColor theming#1344

Draft
JohnVillalovos wants to merge 1 commit intodevelopfrom
jlvillal/issue_1336
Draft

feat(theme): add SVG logo support with currentColor theming#1344
JohnVillalovos wants to merge 1 commit intodevelopfrom
jlvillal/issue_1336

Conversation

@JohnVillalovos
Copy link
Copy Markdown
Collaborator

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:

  • 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 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

Copilot AI review requested due to automatic review settings April 17, 2026 21:23
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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread Web/css/librebooking.css
Comment on lines +84 to +89
.logo {
color: var(--primary);
height: 100%;
display: flex;
align-items: center;
}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
<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" />
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
accept=".png, .gif, .jpg, .jpeg, .svg" />
accept=".png, .gif, .jpg, .svg" />

Copilot uses AI. Check for mistakes.
Comment thread Pages/Page.php
Comment on lines +112 to +121
$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);
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment thread tpl/globalheader.tpl
Comment on lines +128 to +132
{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}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{$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).

Suggested change
{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">

Copilot uses AI. Check for mistakes.
Comment thread tpl/login.tpl
Comment on lines +30 to +34
{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}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{$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.

Suggested change
{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">

Copilot uses AI. Check for mistakes.
Comment thread tpl/maintenance.tpl
Comment on lines +9 to +13
{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}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{$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.

Suggested change
{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">

Copilot uses AI. Check for mistakes.
Comment on lines 118 to 120
$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()));
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@JohnVillalovos JohnVillalovos marked this pull request as draft April 17, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants