From 89303a35e24e4a9c95b03acbf4a7f60ce546590d Mon Sep 17 00:00:00 2001 From: NuPlay <73557895+NuPlay@users.noreply.github.com> Date: Sat, 29 Aug 2026 12:16:29 +0900 Subject: [PATCH 1/2] fix: stop Dynamic Type from discarding the configured fontType Dynamic Type is expressed with the CSS `font` shorthand (`html { font: -apple-system-body; }`), and a shorthand resets every longhand it covers, including `font-family` and `font-style`. Those rules live in `customCSS`, which is emitted after the generated rules, so they always won the cascade. The result was that `supportsDynamicType: true` silently threw away `fontType`, and `.monospaced`, `.italic` and `.customName` all rendered in the default family. Add `Configuration.resolvedCustomCSS`, which appends a font override after the Dynamic Type rules, and use it everywhere `customCSS` was emitted. `customCSS` itself is unchanged, so anything reading it keeps the current behaviour. Also pins down the underlying #63 fix that shipped in v3.0.0: font names must be valid CSS families. The pre-3.0 implementation returned raw PostScript names such as `.AppleSystemUIFontMonospaced-Regular`, which WebKit could not parse as an unquoted family, so it dropped the declaration and fell back to serif. Refs #63 --- Sources/RichText/Models/Configuration.swift | 26 ++++++++++-- Sources/RichText/Views/Webview.swift | 6 +-- .../RichTextSwiftTestingTests.swift | 41 +++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/Sources/RichText/Models/Configuration.swift b/Sources/RichText/Models/Configuration.swift index 41ad4ce..98131c1 100644 --- a/Sources/RichText/Models/Configuration.swift +++ b/Sources/RichText/Models/Configuration.swift @@ -102,6 +102,26 @@ public struct Configuration { } + /// `customCSS` plus the framework-managed overrides that have to win the cascade. + /// + /// Dynamic Type is expressed with the `font` shorthand (`font: -apple-system-body`), and a + /// shorthand resets every longhand it covers, which includes `font-family` and `font-style`. + /// Since `customCSS` is emitted after the generated rules, turning on `supportsDynamicType` + /// silently discarded `fontType`: `.monospaced`, `.italic` and `.customName` all fell back to + /// the default family. Re-assert the configured font after the Dynamic Type rules so the two + /// options can be used together. + public var resolvedCustomCSS: String { + guard supportsDynamicType else { + return customCSS + } + + let fontOverrideCSS = """ + html, body, h1, h2, h3, h4, h5, h6, p { font-family: \(fontType.name); \(fontType.additionalCSSProperties) } + """ + + return customCSS + "\n" + fontOverrideCSS + } + private func backgroundColor(_ isLight: Bool) -> String { let baseColor: String @@ -162,9 +182,9 @@ public struct Configuration { switch scheme { case .light: - return css(isLight: true, alignment: alignment) + "\n" + customCSS + return css(isLight: true, alignment: alignment) + "\n" + resolvedCustomCSS case .dark: - return css(isLight: false, alignment: alignment) + "\n" + customCSS + return css(isLight: false, alignment: alignment) + "\n" + resolvedCustomCSS case .auto: return """ @media (prefers-color-scheme: light) { @@ -173,7 +193,7 @@ public struct Configuration { @media (prefers-color-scheme: dark) { \(css(isLight: false, alignment: alignment)) } - \(customCSS) + \(resolvedCustomCSS) """ } } diff --git a/Sources/RichText/Views/Webview.swift b/Sources/RichText/Views/Webview.swift index 53b1be9..25eb980 100644 --- a/Sources/RichText/Views/Webview.swift +++ b/Sources/RichText/Views/Webview.swift @@ -303,20 +303,20 @@ extension WebView { return String( format: RichTextConstants.cssTemplate, conf.css(isLight: true, alignment: alignment), - conf.customCSS + conf.resolvedCustomCSS ) case .dark: return String( format: RichTextConstants.cssTemplate, conf.css(isLight: false, alignment: alignment), - conf.customCSS + conf.resolvedCustomCSS ) case .auto: return String( format: RichTextConstants.mediaCSSTemplate, conf.css(isLight: true, alignment: alignment), conf.css(isLight: false, alignment: alignment), - conf.customCSS + conf.resolvedCustomCSS ) } } diff --git a/Tests/RichTextTests/RichTextSwiftTestingTests.swift b/Tests/RichTextTests/RichTextSwiftTestingTests.swift index 75137b6..d921533 100644 --- a/Tests/RichTextTests/RichTextSwiftTestingTests.swift +++ b/Tests/RichTextTests/RichTextSwiftTestingTests.swift @@ -155,6 +155,47 @@ struct RichTextAllTests { #expect(FontType.monospaced.name != RichTextConstants.systemFontName) #expect(FontType.italic.name == RichTextConstants.systemFontName) } + + @Test("Font names are valid CSS identifiers or quoted strings") + func fontNamesAreValidCSS() { + // A raw PostScript name such as `.AppleSystemUIFontMonospaced-Regular` is not a + // valid unquoted CSS family, so WebKit dropped the whole declaration and fell back + // to the default serif face. + #expect(!FontType.monospaced.name.hasPrefix(".")) + #expect(!FontType.customName("Custom Font").name.hasPrefix(".")) + #expect(FontType.customName("Custom Font").name == "'Custom Font'") + #expect(FontType.monospaced.name.contains("monospace")) + } + + @Test("Dynamic Type does not discard the configured font type", arguments: [ + FontType.monospaced, + FontType.italic, + FontType.customName("Custom Font") + ]) + func dynamicTypeKeepsFontType(fontType: FontType) { + let config = Configuration(supportsDynamicType: true, fontType: fontType) + let css = config.generateCompleteCSS(colorScheme: .light, alignment: .leading) + + let shorthandRange = css.range(of: "font: -apple-system-body") + let familyRange = css.range(of: "font-family: \(fontType.name)", options: .backwards) + + #expect(shorthandRange != nil) + #expect(familyRange != nil) + + // The `font` shorthand resets `font-family`, so the family has to be re-applied + // after the Dynamic Type rules to survive the cascade. + if let shorthandRange, let familyRange { + #expect(familyRange.lowerBound > shorthandRange.lowerBound) + } + } + + @Test("Custom CSS is untouched when Dynamic Type is off") + func resolvedCustomCSSWithoutDynamicType() { + let config = Configuration(customCSS: "p { color: red; }", fontType: .monospaced) + + #expect(config.resolvedCustomCSS == "p { color: red; }") + #expect(config.resolvedCustomCSS == config.customCSS) + } } @Suite("RichText Initialization Tests") From 621a8fcabb794ef871c7531a74edfdc53a8c2d0f Mon Sep 17 00:00:00 2001 From: NuPlay <73557895+NuPlay@users.noreply.github.com> Date: Sat, 29 Aug 2026 13:00:52 +0900 Subject: [PATCH 2/2] fix: match Dynamic Type paragraph specificity and skip a no-op override Addresses Copilot review feedback on #79. Two problems with the first version of the override: 1. It used a bare `p` selector, specificity (0,0,1). `RichTextConstants.dynamicTypeCSS` styles `p.subheadline`, `p.footnote`, `p.caption1` and `p.caption2` at (0,1,1), so those four paragraph classes still won the cascade and kept resetting `font-family` no matter how late the override was emitted. Name the classes explicitly so specificity ties and source order decides. 2. It was emitted unconditionally. For the default system font the override is a no-op against the Dynamic Type shorthands, but it is appended after `customCSS`, so it would clobber a caller's own `font-family` rule for no benefit. Emit it only when the configured font actually differs, either in family or in the extra properties `.italic` and `.monospaced` add. Tests now compare against the last Dynamic Type shorthand rather than the first, assert the paragraph classes are covered, and assert that no override is emitted for `.system`. --- Sources/RichText/Models/Configuration.swift | 19 +++++++- .../RichTextSwiftTestingTests.swift | 46 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/Sources/RichText/Models/Configuration.swift b/Sources/RichText/Models/Configuration.swift index 98131c1..1640d35 100644 --- a/Sources/RichText/Models/Configuration.swift +++ b/Sources/RichText/Models/Configuration.swift @@ -111,17 +111,32 @@ public struct Configuration { /// the default family. Re-assert the configured font after the Dynamic Type rules so the two /// options can be used together. public var resolvedCustomCSS: String { - guard supportsDynamicType else { + guard supportsDynamicType, requiresFontOverride else { return customCSS } + // `p.subheadline` and friends have to be named explicitly. A bare `p` selector has + // specificity (0,0,1) and would lose to the Dynamic Type `p.subheadline { font: ... }` + // rule at (0,1,1) no matter how late it appears, so those paragraphs would keep + // resetting `font-family`. Matching the specificity lets source order decide, and the + // override is emitted last. let fontOverrideCSS = """ - html, body, h1, h2, h3, h4, h5, h6, p { font-family: \(fontType.name); \(fontType.additionalCSSProperties) } + html, body, h1, h2, h3, h4, h5, h6, p, p.subheadline, p.footnote, p.caption1, p.caption2 { font-family: \(fontType.name); \(fontType.additionalCSSProperties) } """ return customCSS + "\n" + fontOverrideCSS } + /// Whether the configured font actually differs from what the Dynamic Type shorthands + /// already produce. + /// + /// For the default system font the shorthand result and the configured font are the same, + /// so emitting an override would buy nothing and would clobber a caller's own + /// `font-family` rule in `customCSS`. + private var requiresFontOverride: Bool { + fontType.name != RichTextConstants.systemFontName || !fontType.additionalCSSProperties.isEmpty + } + private func backgroundColor(_ isLight: Bool) -> String { let baseColor: String diff --git a/Tests/RichTextTests/RichTextSwiftTestingTests.swift b/Tests/RichTextTests/RichTextSwiftTestingTests.swift index d921533..9c2e1ac 100644 --- a/Tests/RichTextTests/RichTextSwiftTestingTests.swift +++ b/Tests/RichTextTests/RichTextSwiftTestingTests.swift @@ -176,19 +176,53 @@ struct RichTextAllTests { let config = Configuration(supportsDynamicType: true, fontType: fontType) let css = config.generateCompleteCSS(colorScheme: .light, alignment: .leading) - let shorthandRange = css.range(of: "font: -apple-system-body") + // Compare against the *last* Dynamic Type shorthand, not the first: every one of + // them resets `font-family`, so the override only works if it follows all of them. + let lastShorthandRange = css.range(of: "font: -apple-system-", options: .backwards) let familyRange = css.range(of: "font-family: \(fontType.name)", options: .backwards) - #expect(shorthandRange != nil) + #expect(lastShorthandRange != nil) #expect(familyRange != nil) - // The `font` shorthand resets `font-family`, so the family has to be re-applied - // after the Dynamic Type rules to survive the cascade. - if let shorthandRange, let familyRange { - #expect(familyRange.lowerBound > shorthandRange.lowerBound) + if let lastShorthandRange, let familyRange { + #expect(familyRange.lowerBound > lastShorthandRange.lowerBound) } } + @Test("Font override matches the specificity of the Dynamic Type paragraph classes") + func dynamicTypeOverrideCoversParagraphClasses() { + let config = Configuration(supportsDynamicType: true, fontType: .monospaced) + let css = config.generateCompleteCSS(colorScheme: .light, alignment: .leading) + + let overrideLine = css + .split(separator: "\n") + .last { $0.contains("font-family: \(FontType.monospaced.name)") } + + #expect(overrideLine != nil) + + // A bare `p` selector is specificity (0,0,1) and loses to the Dynamic Type + // `p.subheadline { font: ... }` rule at (0,1,1) regardless of source order, so + // those classes have to be named explicitly in the override. + if let overrideLine { + for selector in ["p.subheadline", "p.footnote", "p.caption1", "p.caption2"] { + #expect(overrideLine.contains(selector), "override must also match \(selector)") + } + } + } + + @Test("No font override is emitted for the default system font") + func noFontOverrideForSystemFont() { + // `.system` is exactly what the Dynamic Type shorthands already produce, so an + // override would buy nothing and would clobber the caller's own font-family rule. + let config = Configuration( + customCSS: "body { font-family: Georgia; }", + supportsDynamicType: true + ) + + #expect(config.fontType.name == RichTextConstants.systemFontName) + #expect(config.resolvedCustomCSS == config.customCSS) + } + @Test("Custom CSS is untouched when Dynamic Type is off") func resolvedCustomCSSWithoutDynamicType() { let config = Configuration(customCSS: "p { color: red; }", fontType: .monospaced)