fix: stop icons resolving twice when the fonts come from a Kit or CDN

Core compiles in the bundled FontAwesome Free stylesheets whichever icon
source a forum has configured. Two of them bind the classic family to the
bundled font at `:root`, and a Kit or a Pro CDN sets that same variable from
a stylesheet that can only arrive later — so every icon on the page is
resolved once against the bundled font and then again against the remote
one. The second pass is what readers have been seeing as the icons
flickering shortly after the page appears.

Only the two font bindings are withheld now, and only when the fonts are
not local. The glyph definitions stay: they say which character an icon is,
name no font, and are the same whichever source supplies one. Brands stay
too, since they bind their own family and so never compete with a Kit's.

Withholding them is not sufficient on its own, because the glyph
definitions reach their font through a chain that ends in a hardcoded
`'Font Awesome 7 Free'` — a fallback that belongs to FontAwesome, not to us,
and that wins as soon as nothing else in the chain is set. Remote sources
therefore rebuild the bridge from that chain to the classic family and give
the family a name no browser can resolve. Icons draw nothing at all until
the real font arrives, keeping their box so that nothing moves when the
glyphs appear: briefly absent rather than briefly wrong.

A forum can also name a CDN or Kit and give no URL for it. That source can
deliver nothing, so the bundled fonts are all it has and they are kept —
reading such a forum as remote would leave it with no icons whatsoever
rather than with icons that arrive late.

Two smaller things while in here. The CDN stylesheet no longer carries a
`<noscript>` counterpart: icons are rendered by the SPA, so a browser with
scripting off has no icon elements on the page for it to style, and the
fallback only ever bought such a browser an extra request. And the forced
style help text now says outright that the bundled icons are Free, which
covers only a few hundred icons outside the solid style, so forcing another
style on them leaves most icons blank.

Author: imorland

framework/core/less/common/Iconography.less

@@ -1,7 +1,61 @@
+// Which glyph each icon is, and how icons size, stack and spin. This names no
+// font and is the same whichever source supplies one, so it is always needed —
+// without it an icon has nothing to draw.
 @import (inline) "fontawesome.css";
+
+// Brands bind their own family rather than the classic one, so they never
+// compete with a Kit's override and can stay in unconditionally.
 @import (inline) "brands.css";
-@import (inline) "regular.css";
-@import (inline) "solid.css";
+
+// These two bind the classic family to the bundled Free font at `:root`. A Kit,
+// or a Pro CDN, sets that same variable from a stylesheet that necessarily
+// arrives later, so keeping these when the icons are remote means every icon is
+// resolved twice — once against the bundled font, then again once the remote one
+// lands. Readers see that second pass as the icons flickering, and under a
+// forced style the bundled Free font is missing most of the glyphs, so the
+// first pass shows empty boxes.
+//
+// `@fa-bundled-fonts` comes from FontAwesome::needsBundledFonts().
+.fa-bundled-font-faces() when (@fa-bundled-fonts = true) {
+  @import (inline) "regular.css";
+  @import (inline) "solid.css";
+}
+.fa-bundled-font-faces();
+
+// Withholding those two is not enough on its own. The glyph definitions resolve
+// their font as
+//
+//   var(--fa-family, var(--fa-style-family, 'Font Awesome 7 Free'))
+//
+// a chain that never consults `--fa-family-classic`. Bridging the two was the
+// job of the stylesheets withheld above, so without a replacement the hardcoded
+// literal at the end wins and the icons resolve to the bundled font exactly as
+// before.
+//
+// So rebuild the bridge, and point it at a family no browser can resolve. Icons
+// draw nothing until the real font arrives, keeping their box so the layout does
+// not move when the glyphs appear — briefly absent rather than briefly wrong.
+//
+// The placeholder belongs on `:root` and the bridge on the classes, in that
+// order of specificity: a Kit publishes its font by setting `--fa-family-classic`
+// at `:root`, which then replaces the placeholder and flows through the bridge on
+// its own. Putting the placeholder on the classes instead would outrank the Kit
+// and leave the icons blank for good.
+.fa-remote-pending-font() when (@fa-bundled-fonts = false) {
+  :root {
+    --fa-family-classic: "FontAwesome Pending";
+  }
+
+  .fa-solid,
+  .fa-regular,
+  .fa-classic,
+  .fas,
+  .far,
+  .fa {
+    --fa-family: var(--fa-family-classic);
+  }
+}
+.fa-remote-pending-font();
 
 // FontAwesome 7 (and its Kit JS) sets `display: var(--fa-display, inline-block)` on
 // .fas/.far/.fab etc. When a Kit is loaded its dynamically-injected stylesheet lands

framework/core/locale/core.yml

@@ -57,7 +57,7 @@ core:
         kit_url_label: Kit URL
         kit_url_help: Your FontAwesome Kit script URL (from kit.fontawesome.com)
         forced_style_label: Forced Icon Style
-        forced_style_help: "Force every icon to a particular FontAwesome style, e.g. \"fa-duotone fa-light\" or \"fa-regular\". Replaces the style declared by core and extensions (brand icons are never changed). The style must be available in your icon source — most styles require FontAwesome Pro. Leave empty to disable."
+        forced_style_help: "Force every icon to a particular FontAwesome style, e.g. \"fa-duotone fa-light\" or \"fa-regular\". Replaces the style declared by core and extensions (brand icons are never changed). Your icon source has to be able to supply the style: the bundled icons are FontAwesome Free, which only covers a few hundred icons outside the solid style, so forcing anything else on them leaves most icons blank. Every style is available with a FontAwesome Pro CDN or Kit. Leave empty to disable."
         config_override:
           label: Config Override
           help: FontAwesome settings are currently set in your config.php file and cannot be changed here.

framework/core/src/Foundation/FontAwesome.php

@@ -66,6 +66,35 @@ public function useLocalFonts(): bool
         return $this->source() === self::SOURCE_LOCAL;
     }
 
+    /**
+     * Whether the bundled font files are what the icons will actually be drawn
+     * from.
+     *
+     * This is not quite the same question as `useLocalFonts()`. A forum can name
+     * a CDN or a Kit and give no URL for it, in which case that source cannot
+     * deliver anything and the bundled fonts are all there is. Treating such a
+     * forum as remote would leave it with no icons at all, rather than with
+     * icons that arrive late.
+     */
+    public function needsBundledFonts(): bool
+    {
+        if ($this->useLocalFonts()) {
+            return true;
+        }
+
+        if ($this->useCdn()) {
+            return $this->cdnUrl() === null;
+        }
+
+        if ($this->useKit()) {
+            return $this->kitUrl() === null;
+        }
+
+        // An unrecognised source names nothing we can load, so the bundled
+        // fonts remain the only ones available.
+        return true;
+    }
+
     public function useCdn(): bool
     {
         return $this->source() === self::SOURCE_CDN;

framework/core/src/Frontend/FrontendServiceProvider.php

@@ -122,10 +122,12 @@ public function register(): void
                             }
                             // Load asynchronously — FA icons are only rendered after JS boots the
                             // SPA, so there is no FOUC risk from deferring this stylesheet.
-                            // The <noscript> fallback covers JS-disabled browsers.
-                            $escaped = e($cdnUrl);
-                            $document->head[] = '<link rel="preload" href="'.$escaped.'" as="style" crossorigin="anonymous" onload="this.onload=null;this.rel=\'stylesheet\'">'
-                                .'<noscript><link rel="stylesheet" href="'.$escaped.'" crossorigin="anonymous"></noscript>';
+                            //
+                            // Deliberately without a <noscript> counterpart: the icons are
+                            // rendered by the SPA, so a browser with scripting off has no icon
+                            // elements on the page for this stylesheet to style. The fallback
+                            // only ever bought such a browser an extra request.
+                            $document->head[] = '<link rel="preload" href="'.e($cdnUrl).'" as="style" crossorigin="anonymous" onload="this.onload=null;this.rel=\'stylesheet\'">';
                         }
                     } elseif ($fontAwesome->useKit()) {
                         $kitUrl = $fontAwesome->kitUrl();
@@ -247,7 +249,16 @@ function (Container $container) {
         $this->container->singleton(
             'flarum.less.custom_variables',
             function (Container $container) {
-                return [];
+                return [
+                    // Whether to compile in the bundled Free font bindings. They
+                    // name a font at `:root`, so leaving them in when the icons
+                    // come from a Kit or CDN means every icon is resolved once
+                    // against the bundled font and again against the remote one
+                    // — which is what shows up as the icons flickering.
+                    'fa-bundled-fonts' => function () use ($container) {
+                        return $container->make(FontAwesome::class)->needsBundledFonts() ? 'true' : 'false';
+                    },
+                ];
             }
         );
 

framework/core/tests/integration/frontend/FontAwesomeLoadingTest.php

@@ -69,8 +69,10 @@ public function fontawesome_cdn_loads_css_instead_of_local_fonts()
         // Should load CDN CSS asynchronously (preload + onload swap)
         $this->assertStringContainsString('<link rel="preload" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.2/css/all.min.css" as="style" crossorigin="anonymous" onload="this.onload=null;this.rel=\'stylesheet\'">', $body);
 
-        // Should include noscript fallback
-        $this->assertStringContainsString('<noscript><link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.2/css/all.min.css" crossorigin="anonymous"></noscript>', $body);
+        // No `<noscript>` counterpart. Icons are rendered by the SPA, so with
+        // scripting off the page contains no icon elements for a stylesheet to
+        // style — the fallback only ever cost a request.
+        $this->assertStringNotContainsString('<noscript><link rel="stylesheet"', $body);
 
         // Should not contain local font preloads
         $this->assertStringNotContainsString('fa-solid-900.woff2', $body);
@@ -164,8 +166,8 @@ public function config_override_takes_precedence_over_database_settings()
         // Should load config CDN CSS asynchronously (preload + onload swap)
         $this->assertStringContainsString('<link rel="preload" href="https://config.example.com/fontawesome.css" as="style" crossorigin="anonymous" onload="this.onload=null;this.rel=\'stylesheet\'">', $body);
 
-        // Should include noscript fallback
-        $this->assertStringContainsString('<noscript><link rel="stylesheet" href="https://config.example.com/fontawesome.css" crossorigin="anonymous"></noscript>', $body);
+        // No `<noscript>` counterpart — see the CDN test above.
+        $this->assertStringNotContainsString('<noscript><link rel="stylesheet"', $body);
 
         // Should not contain local font preloads
         $this->assertStringNotContainsString('fa-solid-900.woff2', $body);