From 3227a69d50de0fbc0e4660d054974b5e12e3735a Mon Sep 17 00:00:00 2001
From: NuPlay <73557895+NuPlay@users.noreply.github.com>
Date: Sat, 29 Aug 2026 18:10:38 +0900
Subject: [PATCH] refactor: build CSS and HTML by interpolation instead of
 String(format:)

The templates were printf format strings rendered with `String(format:)`, which
treats `%` as the start of a conversion. CSS is full of literal percent signs, so
every one of them had to be written `%%`, and forgetting one was silent. That
single choice produced three separate defects:

- `imageCSS` was missing its escapes, so `max-width: 100%` shipped as
  `max-width: 100` - an invalid length the browser drops - and images were not
  constrained to the web view width. Reported in December 2025, fixed in #76.
- `iframeCSS` used `%d` for a Swift `Int`, reading 32 bits of a 64-bit value.
- `htmlTemplate` took seven positional arguments whose order nothing enforced,
  which needed a dedicated test to pin down.

Interpolation removes all three classes at once: percent signs are literal, the
compiler checks the types, and arguments are named. The container id and the two
script message handler names are constants inside `htmlDocument(css:body:)`, so
five of those seven positional arguments no longer exist at all.

The seven format strings stay as deprecated public API and still work for anyone
using them directly. Output is unchanged: each builder was diffed against the
format string it replaces and all seven render byte-identical results, and the
generated document was re-checked in a browser for JS syntax and for the
`<details>` height resync.

The regression test that scanned the format strings for unescaped percent signs
is removed with them; it existed only to police a footgun that no longer has a
live call site. Assertions on the generated output are kept.
---
 Sources/RichText/Models/Configuration.swift   |  19 +-
 .../RichText/Models/RichTextConstants.swift   | 185 ++++++++++++++++++
 Sources/RichText/Views/Webview.swift          |  34 ++--
 .../RichTextSwiftTestingTests.swift           | 111 +++--------
 4 files changed, 231 insertions(+), 118 deletions(-)

diff --git a/Sources/RichText/Models/Configuration.swift b/Sources/RichText/Models/Configuration.swift
index 2df945b..2730d8b 100644
--- a/Sources/RichText/Models/Configuration.swift
+++ b/Sources/RichText/Models/Configuration.swift
@@ -178,17 +178,16 @@ public struct Configuration {
     ///   - alignment: Text alignment preference
     /// - Returns: Generated CSS string
     public func css(isLight: Bool, alignment: TextAlignment) -> String {
-        let imageCSS = String(format: RichTextConstants.imageCSS, "\(imageRadius)")
-        let textCSS = String(
-            format: RichTextConstants.textCSS,
-            alignment.htmlDescription,
-            "\(lineHeight)",
-            fontType.name,
-            fontColor.value(isLight),
-            backgroundColor(isLight)
+        let imageCSS = RichTextConstants.imageCSS(radius: imageRadius)
+        let textCSS = RichTextConstants.textCSS(
+            alignment: alignment.htmlDescription,
+            lineHeight: lineHeight,
+            fontFamily: fontType.name,
+            color: fontColor.value(isLight),
+            backgroundColor: backgroundColor(isLight)
         )
-        let iframeCSS = String(format: RichTextConstants.iframeCSS, RichTextConstants.iframeHeight)
-        let linkCSS = String(format: RichTextConstants.linkCSS, linkColor.value(isLight))
+        let iframeCSS = RichTextConstants.iframeCSS()
+        let linkCSS = RichTextConstants.linkCSS(color: linkColor.value(isLight))
         
         // Add font-specific CSS properties
         let fontSpecificCSS = !fontType.additionalCSSProperties.isEmpty ? 
diff --git a/Sources/RichText/Models/RichTextConstants.swift b/Sources/RichText/Models/RichTextConstants.swift
index 7ff6a79..e07e4b1 100644
--- a/Sources/RichText/Models/RichTextConstants.swift
+++ b/Sources/RichText/Models/RichTextConstants.swift
@@ -54,12 +54,16 @@ public struct RichTextConstants {
     // `loading: lazy` was also dropped: `loading` is an HTML attribute, not a CSS property,
     // so no browser ever applied it. Real lazy loading would have to be set on the `<img>`
     // elements themselves.
+    @available(*, deprecated, message: "Use the imageCSS(radius:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let imageCSS = "img{height:auto; max-width: 100%%; width:auto;margin-bottom:5px; border-radius: %@px;}"
+    @available(*, deprecated, message: "Use the textCSS(alignment:lineHeight:fontFamily:color:backgroundColor:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let textCSS = "h1, h2, h3, h4, h5, h6, p, div, dl, ol, ul, pre, blockquote, figure, figcaption, details, summary, article, section, aside, header, footer, nav, main {text-align:%@; line-height: %@%%; font-family: %@; color: %@; background-color: %@; word-wrap: break-word; }"
     // `%ld`, not `%d`: `iframeHeight` is a Swift `Int`, which is 64-bit on every platform this
     // package supports, while `%d` reads only 32 bits of it. Matches the `%02lX` spelling
     // already used in `Color+Extension`.
+    @available(*, deprecated, message: "Use the iframeCSS(height:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let iframeCSS = "iframe{width:100%%; height:%ldpx; border: none;}"
+    @available(*, deprecated, message: "Use the linkCSS(color:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let linkCSS = "a:link {color: %@; transition: color 0.2s ease;}"
     public static let linkDecorationCSS = "A {text-decoration: none;} A:hover {text-decoration: underline;}"
     @available(*, deprecated, message: "Unused. Body margins are emitted directly by cssTemplate and mediaCSSTemplate, and this constant was never applied, so its -webkit-text-size-adjust rule never took effect either.")
@@ -101,6 +105,7 @@ public struct RichTextConstants {
         """
     
     // MARK: - HTML Templates (v3.0.0 - Modern, accessible markup)
+    @available(*, deprecated, message: "Use the htmlDocument(css:body:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let htmlTemplate = """
         <HTML>
         <head>
@@ -193,6 +198,7 @@ public struct RichTextConstants {
         </HTML>
         """
     
+    @available(*, deprecated, message: "Use the styleDocument(css:customCSS:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let cssTemplate = """
         <style type='text/css'>
             %@
@@ -205,6 +211,7 @@ public struct RichTextConstants {
         <BODY>
         """
     
+    @available(*, deprecated, message: "Use the styleDocument(lightCSS:darkCSS:customCSS:) builder instead. This is a printf format string, so every literal percent sign has to be written as %%, which is what caused images to lose their max-width.")
     public static let mediaCSSTemplate = """
         <style type='text/css'>
         @media (prefers-color-scheme: light) {
@@ -222,3 +229,181 @@ public struct RichTextConstants {
         <BODY>
         """
 }
+
+// MARK: - Builders
+
+/// Interpolated replacements for the printf format strings above.
+///
+/// The format strings were rendered with `String(format:)`, which treats `%` as the start of
+/// a conversion. Every literal percent sign in CSS - and CSS is full of them - therefore had
+/// to be written `%%`. Missing one was silent and produced invalid CSS: `max-width: 100%`
+/// became `max-width: 100`, which the browser drops. That is what left images unconstrained
+/// by the web view width for months.
+///
+/// The same mechanism cost two more defects: `%d` was reading 32 bits of a 64-bit Swift `Int`,
+/// and the HTML template took seven positional arguments whose order nothing enforced.
+///
+/// String interpolation removes all three classes at once: percent signs are literal, the
+/// compiler checks the types, and arguments are named.
+extension RichTextConstants {
+
+    /// Responsive image sizing.
+    public static func imageCSS(radius: CGFloat) -> String {
+        "img{height:auto; max-width: 100%; width:auto;margin-bottom:5px; border-radius: \(radius)px;}"
+    }
+
+    /// Block level text styling.
+    public static func textCSS(
+        alignment: String,
+        lineHeight: CGFloat,
+        fontFamily: String,
+        color: String,
+        backgroundColor: String
+    ) -> String {
+        "h1, h2, h3, h4, h5, h6, p, div, dl, ol, ul, pre, blockquote, figure, figcaption, details, summary, article, section, aside, header, footer, nav, main {text-align:\(alignment); line-height: \(lineHeight)%; font-family: \(fontFamily); color: \(color); background-color: \(backgroundColor); word-wrap: break-word; }"
+    }
+
+    /// Embedded iframe sizing.
+    public static func iframeCSS(height: Int = RichTextConstants.iframeHeight) -> String {
+        "iframe{width:100%; height:\(height)px; border: none;}"
+    }
+
+    /// Link colouring.
+    public static func linkCSS(color: String) -> String {
+        "a:link {color: \(color); transition: color 0.2s ease;}"
+    }
+
+    /// A `<style>` block for a single colour scheme, followed by the opening `<BODY>` tag.
+    public static func styleDocument(css: String, customCSS: String) -> String {
+        """
+        <style type='text/css'>
+            \(css)
+            \(customCSS)
+            body {
+                margin: 0;
+                padding: 0;
+            }
+        </style>
+        <BODY>
+        """
+    }
+
+    /// A `<style>` block that switches on `prefers-color-scheme`, followed by `<BODY>`.
+    public static func styleDocument(lightCSS: String, darkCSS: String, customCSS: String) -> String {
+        """
+        <style type='text/css'>
+        @media (prefers-color-scheme: light) {
+            \(lightCSS)
+        }
+        @media (prefers-color-scheme: dark) {
+            \(darkCSS)
+        }
+        \(customCSS)
+        body {
+            margin: 0;
+            padding: 0;
+        }
+        </style>
+        <BODY>
+        """
+    }
+
+    /// The complete HTML document handed to the web view.
+    ///
+    /// The container id and the two script message handler names are constants, so unlike the
+    /// old seven-placeholder format string there is nothing here for a caller to get out of
+    /// order.
+    public static func htmlDocument(css: String, body: String) -> String {
+        """
+        <HTML>
+        <head>
+            <meta name='viewport' content='width=device-width, shrink-to-fit=YES, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no'>
+        </head>
+        \(css)
+        <div id="\(richTextContainerID)">\(body)</div>
+        </BODY>
+        <script>
+            var richTextHeightHandler = '\(heightNotificationHandler)';
+            var richTextContainer = document.getElementById('\(richTextContainerID)');
+            var richTextLastReportedHeight = -1;
+
+            function syncHeight() {
+              if (!richTextContainer) { return; }
+              if (!window.webkit || !window.webkit.messageHandlers) { return; }
+
+              var handler = window.webkit.messageHandlers[richTextHeightHandler];
+              if (!handler) { return; }
+
+              var boundingHeight = richTextContainer.getBoundingClientRect().height;
+              var height = Math.ceil(Math.max(richTextContainer.scrollHeight, boundingHeight));
+
+              // Skip redundant round trips; the native side re-renders on every update.
+              if (height === richTextLastReportedHeight) { return; }
+              richTextLastReportedHeight = height;
+              handler.postMessage(height);
+            }
+
+            function handleMediaClick(element, type) {
+              if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.\(mediaClickHandler)) {
+                window.webkit.messageHandlers.\(mediaClickHandler).postMessage({
+                  type: type,
+                  src: element.src || element.getAttribute('src')
+                });
+              }
+            }
+
+            function attachMediaHandlers() {
+              var imgs = document.getElementsByTagName('img');
+              for (var i = 0; i < imgs.length; i++) {
+                if (imgs[i].dataset.richTextBound) { continue; }
+                imgs[i].dataset.richTextBound = '1';
+                imgs[i].addEventListener('load', syncHeight);
+                imgs[i].addEventListener('error', syncHeight);
+                imgs[i].onclick = function () {
+                  handleMediaClick(this, 'image');
+                };
+              }
+
+              var videos = document.getElementsByTagName('video');
+              for (var j = 0; j < videos.length; j++) {
+                if (videos[j].dataset.richTextBound) { continue; }
+                videos[j].dataset.richTextBound = '1';
+                videos[j].addEventListener('loadedmetadata', syncHeight);
+                videos[j].onclick = function () {
+                  handleMediaClick(this, 'video');
+                };
+              }
+            }
+
+            attachMediaHandlers();
+            syncHeight();
+
+            // The rendered height keeps changing after the first layout pass: images and
+            // web fonts finish loading, <details> elements are toggled open, the device is
+            // rotated, Dynamic Type changes. A one-shot measurement on window.onload misses
+            // all of those and the content ends up clipped, so observe the container instead.
+            if (window.ResizeObserver && richTextContainer) {
+              new ResizeObserver(syncHeight).observe(richTextContainer);
+            }
+
+            if (window.MutationObserver && richTextContainer) {
+              new MutationObserver(function () {
+                attachMediaHandlers();
+                syncHeight();
+              }).observe(richTextContainer, { childList: true, subtree: true });
+            }
+
+            // `toggle` does not bubble, so it has to be observed during the capture phase.
+            document.addEventListener('toggle', syncHeight, true);
+            window.addEventListener('load', syncHeight);
+            window.addEventListener('resize', syncHeight);
+            window.addEventListener('orientationchange', syncHeight);
+
+            if (document.fonts && document.fonts.ready) {
+              document.fonts.ready.then(syncHeight);
+            }
+        </script>
+        </HTML>
+        """
+    }
+}
diff --git a/Sources/RichText/Views/Webview.swift b/Sources/RichText/Views/Webview.swift
index bfb5045..01c680e 100644
--- a/Sources/RichText/Views/Webview.swift
+++ b/Sources/RichText/Views/Webview.swift
@@ -341,16 +341,7 @@ extension WebView {
     /// Generates the complete HTML string for the WebView
     /// - Returns: Complete HTML document string
     func generateHTML() -> String {
-        return String(
-            format: RichTextConstants.htmlTemplate,
-            generateCSS(),
-            RichTextConstants.richTextContainerID,
-            html,
-            RichTextConstants.heightNotificationHandler,
-            RichTextConstants.richTextContainerID,
-            RichTextConstants.mediaClickHandler,
-            RichTextConstants.mediaClickHandler
-        )
+        return RichTextConstants.htmlDocument(css: generateCSS(), body: html)
     }
     
     /// Generates CSS styles based on color scheme configuration
@@ -358,23 +349,20 @@ extension WebView {
     func generateCSS() -> String {
         switch conf.colorScheme {
         case .light:
-            return String(
-                format: RichTextConstants.cssTemplate,
-                conf.css(isLight: true, alignment: alignment),
-                conf.resolvedCustomCSS
+            return RichTextConstants.styleDocument(
+                css: conf.css(isLight: true, alignment: alignment),
+                customCSS: conf.resolvedCustomCSS
             )
         case .dark:
-            return String(
-                format: RichTextConstants.cssTemplate,
-                conf.css(isLight: false, alignment: alignment),
-                conf.resolvedCustomCSS
+            return RichTextConstants.styleDocument(
+                css: conf.css(isLight: false, alignment: alignment),
+                customCSS: conf.resolvedCustomCSS
             )
         case .auto:
-            return String(
-                format: RichTextConstants.mediaCSSTemplate,
-                conf.css(isLight: true, alignment: alignment),
-                conf.css(isLight: false, alignment: alignment),
-                conf.resolvedCustomCSS
+            return RichTextConstants.styleDocument(
+                lightCSS: conf.css(isLight: true, alignment: alignment),
+                darkCSS: conf.css(isLight: false, alignment: alignment),
+                customCSS: conf.resolvedCustomCSS
             )
         }
     }
diff --git a/Tests/RichTextTests/RichTextSwiftTestingTests.swift b/Tests/RichTextTests/RichTextSwiftTestingTests.swift
index 64ab07c..9b0700c 100644
--- a/Tests/RichTextTests/RichTextSwiftTestingTests.swift
+++ b/Tests/RichTextTests/RichTextSwiftTestingTests.swift
@@ -87,9 +87,8 @@ struct RichTextAllTests {
             #expect(css.contains("max-width: 100%"))
             #expect(css.contains("height:auto"))
 
-            // `iframeHeight` is a Swift `Int`, so the format string has to use `%ld`.
-            // With `%d` only the low 32 bits are read, which happens to work for small
-            // values and silently would not for large ones.
+            // The iframe height is interpolated, so there is no `%d` reading 32 bits of a
+            // 64-bit `Int` any more.
             #expect(css.contains("height:\(RichTextConstants.iframeHeight)px"))
         }
 
@@ -120,15 +119,15 @@ struct RichTextAllTests {
             #expect(css.contains(expectedAlignment))
         }
 
-        @Test("Percent signs survive String(format:) based CSS generation")
-        func percentSignsSurviveFormatting() {
+        @Test("Percentage lengths reach the generated CSS intact")
+        func percentageLengthsReachTheCSS() {
             let css = Configuration(lineHeight: 150, imageRadius: 5).css(isLight: true, alignment: .leading)
 
-            // The CSS constants that carry substitutions are rendered with `String(format:)`,
-            // which consumes a bare `%`. A literal percent sign therefore has to be written as
-            // `%%` in those constants. Missing one is silent: `max-width: 100%` becomes
-            // `max-width: 100`, an invalid length that WebKit drops, and images then overflow
-            // the web view instead of being constrained to its width.
+            // These used to go through `String(format:)`, which consumes a bare `%`, so every
+            // literal percent sign had to be written `%%`. Missing one was silent:
+            // `max-width: 100%` became `max-width: 100`, an invalid length that WebKit drops,
+            // and images overflowed the web view. The builders interpolate instead, so the
+            // escaping cannot be forgotten - this just pins the output down.
             #expect(css.contains("max-width: 100%"))
             #expect(css.contains("width:100%;"))
             #expect(css.contains("line-height: 150.0%"))
@@ -136,51 +135,6 @@ struct RichTextAllTests {
             #expect(!css.contains("max-width: 100;"))
             #expect(!css.contains("width:100;"))
         }
-
-        @Test("Percent based CSS constants are escaped for String(format:)", arguments: [
-            ("imageCSS", RichTextConstants.imageCSS),
-            ("textCSS", RichTextConstants.textCSS),
-            ("iframeCSS", RichTextConstants.iframeCSS),
-            ("linkCSS", RichTextConstants.linkCSS),
-            ("cssTemplate", RichTextConstants.cssTemplate),
-            ("mediaCSSTemplate", RichTextConstants.mediaCSSTemplate),
-            ("htmlTemplate", RichTextConstants.htmlTemplate)
-        ])
-        func percentBasedConstantsAreEscaped(name: String, constant: String) {
-            // Covers every constant that `Configuration.css(isLight:alignment:)`,
-            // `WebView.generateCSS()` and `WebView.generateHTML()` pass to `String(format:)`.
-            //
-            // Look for the shape the bug actually takes rather than whitelisting conversion
-            // characters. A literal percent sign in CSS is always followed by a delimiter -
-            // `100%;`, `100% }`, `100%,` - while a conversion is followed by a specifier or a
-            // length modifier. Checking the delimiters catches a missing `%%` without pinning
-            // down which conversions the constants are allowed to use, so `%d` can still be
-            // widened to `%ld` later without this test standing in the way.
-            let cssDelimiters: Set<Character> = [";", "}", ",", ")", " ", "\n", "\t"]
-            let characters = Array(constant)
-            var index = 0
-
-            while index < characters.count {
-                guard characters[index] == "%" else {
-                    index += 1
-                    continue
-                }
-
-                let next = index + 1 < characters.count ? characters[index + 1] : nil
-
-                // `%%` is an escaped literal percent, which is exactly what we want to see.
-                if next == "%" {
-                    index += 2
-                    continue
-                }
-
-                #expect(
-                    next != nil && !cssDelimiters.contains(next!),
-                    "Unescaped percent sign in \(name): a literal % must be written as %%"
-                )
-                index += 1
-            }
-        }
     }
 
     @Suite("ColorSet Tests")
@@ -385,44 +339,31 @@ struct RichTextAllTests {
             #expect(!RichTextConstants.systemFontName.isEmpty)
         }
 
-        @Test("HTML template keeps measuring the content after the first layout pass")
-        func htmlTemplateObservesHeightChanges() {
-            let template = RichTextConstants.htmlTemplate
+        @Test("Generated document keeps measuring the content after the first layout pass")
+        func generatedDocumentObservesHeightChanges() {
+            let document = RichTextConstants.htmlDocument(css: "<style></style>", body: "<p>Hello</p>")
 
             // A one-shot `window.onload` measurement misses late images, web fonts and
             // `<details>` toggles, which is what caused the content to be clipped.
-            #expect(template.contains("ResizeObserver"))
-            #expect(template.contains("MutationObserver"))
-            #expect(template.contains("document.addEventListener('toggle', syncHeight, true)"))
-            #expect(template.contains("document.fonts.ready"))
-            #expect(!template.contains("window.onload = function"))
-        }
-
-        @Test("HTML template placeholders stay in the order WebView.generateHTML supplies them")
-        func htmlTemplatePlaceholderOrder() {
-            let placeholderCount = RichTextConstants.htmlTemplate.components(separatedBy: "%@").count - 1
-
-            // css, container id, html body, height handler, container id, media handler, media handler
-            #expect(placeholderCount == 7)
-
-            // Mirrors the argument list in `WebView.generateHTML()`. `String(format:)` is
-            // positional, so reordering the placeholders inside the template silently
-            // produces a broken document; this pins the mapping down.
-            let document = String(
-                format: RichTextConstants.htmlTemplate,
-                "<style type='text/css'></style>",
-                RichTextConstants.richTextContainerID,
-                "<p>Hello</p>",
-                RichTextConstants.heightNotificationHandler,
-                RichTextConstants.richTextContainerID,
-                RichTextConstants.mediaClickHandler,
-                RichTextConstants.mediaClickHandler
-            )
+            #expect(document.contains("ResizeObserver"))
+            #expect(document.contains("MutationObserver"))
+            #expect(document.contains("document.addEventListener('toggle', syncHeight, true)"))
+            #expect(document.contains("document.fonts.ready"))
+            #expect(!document.contains("window.onload = function"))
+        }
+
+        @Test("Generated document wires the script bridge to the rendered container")
+        func generatedDocumentWiresTheBridge() {
+            let document = RichTextConstants.htmlDocument(css: "<style></style>", body: "<p>Hello</p>")
 
             #expect(document.contains("<div id=\"\(RichTextConstants.richTextContainerID)\"><p>Hello</p></div>"))
             #expect(document.contains("var richTextHeightHandler = '\(RichTextConstants.heightNotificationHandler)'"))
             #expect(document.contains("document.getElementById('\(RichTextConstants.richTextContainerID)')"))
             #expect(document.contains("window.webkit.messageHandlers.\(RichTextConstants.mediaClickHandler).postMessage({"))
+
+            // The container id and the handler names are constants inside the builder, so
+            // there are no positional arguments left for a caller to get out of order.
+            #expect(!document.contains("%@"))
         }
     }