Skip to content

feat: app icon, .ico favicon, web app manifest, and logos and icons set by themes - #5319

Merged
imorland merged 2 commits into
2.xfrom
im/forum-icons
Oct 28, 2026
Merged

imorland merged 2 commits into
2.xfrom
im/forum-icons

Conversation

@imorland

@imorland imorland commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Fixes: no issue filed.

Changes proposed in this pull request:

A forum can only set a favicon, which is stored as a 64px PNG and linked as a shortcut icon. There is no Apple touch icon or web app manifest, so phones show a screenshot or a letter when someone adds the forum to their home screen. .ico favicons are refused by the validator. A theme can't set any of its logos or icons from code.

  • App icon: a new upload on the Appearance page, square and at least 512px. Flarum draws Apple's touch icon (180px, opaque on a background colour the admin picks), 192px and 512px manifest icons, and a 512px maskable icon padded to Android's safe zone. The page previews it as iPhones and Android show it, and changing the background colour redraws the icons from the kept source.
  • Favicon: an upload also produces a 16/32/48px .ico, which the page links, and .ico files can be uploaded. Without a favicon, the app icon is used.
  • Web app manifest at /manifest, served by Flarum so it works on any web server and stays on the forum's domain when assets are on S3 or a CDN. It opens the forum in the browser; Extend\Theme::manifest() lets an extension such as fof/pwa change that. /favicon.ico redirects to the icon for crawlers and feed readers.
  • Extend\Theme can set every logo and icon: logo(), logoDarkMode(), emailLogo(), favicon() (with an optional SVG) and appIcon(). They go through the same pipeline as uploads, and the Appearance page shows them locked. logo(height, quality) from feat: store logos taller and at a higher quality #5309, not yet released, becomes logoSize().

Every size is drawn from the source, never from a smaller copy, and never enlarged. Icons are PNGs at the highest compression, and every file is stored with an explicit content type and a long-lived cache header, as names change whenever the image does.

Docs: flarum/docs#693

Reviewers should focus on:

  • Flarum\Forum\Branding\Branding is the one place logo and icon URLs come from; ForumResource, the page head, emails, the standalone pages and discussion link favicons all ask it.
  • ThemeImages draws a theme's images once, and again only when a file or option changes, keeping what it drew in a setting. A request compares file sizes and modification times once; an image that can't be drawn is logged and left out rather than retried on every request.
  • New constructor arguments are optional and last, so existing subclasses keep working.
  • fof/pwa links its own manifest after core's, so browsers use core's when there is an app icon until fof/pwa builds on Theme::manifest() instead (Build on core's web app manifest and app icon (Flarum 2.1) FriendsOfFlarum/pwa#36).
  • Flarum's nginx config sends .ico requests straight to the filesystem, so /favicon.ico only reaches Flarum there with a config change in flarum/flarum ([2.x] Serve /favicon.ico through Flarum in the nginx config shipped with flarum/flarum #5320). Browsers use the page's links either way.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Frontend changes: tests are green (run yarn test in js/).
  • Frontend changes: tests have been added, or are not appropriate here.
  • Backend changes: tests are green (run composer test).
  • Backend changes: tests have been added, or are not appropriate here.
  • Where applicable, changes are suitable for all supported database drivers (MySQL, MariaDB, PostgreSQL, SQLite). (No database changes.)
  • Core developer confirmed locally this works as intended.
  • The description above is written by me and describes what this pull request actually does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants