From 0eed8cc518b95e70b73a6611ab3d4736a05bab04 Mon Sep 17 00:00:00 2001 From: NuPlay <73557895+NuPlay@users.noreply.github.com> Date: Sat, 29 Aug 2026 18:13:43 +0900 Subject: [PATCH] chore: drop dead CSS declarations and deprecate no-op API `imageCSS` declared `loading: lazy`. `loading` is an HTML attribute, not a CSS property, so every browser dropped the declaration; it only ever looked like lazy loading was enabled. Enabling it for real would mean setting the attribute on the `` elements. `imageCSS` also carried percentage `min-height`/`max-height`. Both resolved against a containing block of `auto` height, so they were no-ops that contributed nothing to the responsive behaviour, and `min-height: 100%` would have stretched every image to the full container height as soon as a definite height appeared. `max-width: 100%` with `height: auto` is what does the work. `iframeCSS` used `%d` for `iframeHeight`, which is a Swift `Int` and therefore 64-bit on every platform this package supports, while `%d` reads 32 bits. It works for the current value of 250 and would silently not for a large one. `Color+Extension` already spells the same thing `%02lX`, so `%ld` matches. `bodyCSS` is referenced nowhere. Body margins are emitted directly by `cssTemplate` and `mediaCSSTemplate`, which means its `-webkit-text-size-adjust` rule has never been applied either. `Configuration.isColorsImportant` is recorded but never read during CSS generation - `!important` comes entirely from `ColorSet.isImportant` - so passing `isColorsImportant: .all` silently did nothing. Both are deprecated rather than deleted, since they are public API. Behaviour is unchanged and the colour preference still round-trips through internal storage, so the framework does not warn on its own API. A new test pins down which of the two paths actually reaches the CSS. --- .../Extensions/RichText+Extension.swift | 2 +- Sources/RichText/Models/Configuration.swift | 24 +++++++-- .../RichText/Models/RichTextConstants.swift | 19 ++++++- .../RichTextSwiftTestingTests.swift | 52 ++++++++++++++++--- 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/Sources/RichText/Extensions/RichText+Extension.swift b/Sources/RichText/Extensions/RichText+Extension.swift index b609dc5..082df85 100644 --- a/Sources/RichText/Extensions/RichText+Extension.swift +++ b/Sources/RichText/Extensions/RichText+Extension.swift @@ -160,7 +160,7 @@ extension RichText { public func colorPreference(forceColor: ColorPreference) -> RichText { var result = self - result.configuration.isColorsImportant = forceColor + result.configuration.storedColorPreference = forceColor switch forceColor { case .all: diff --git a/Sources/RichText/Models/Configuration.swift b/Sources/RichText/Models/Configuration.swift index 1640d35..2df945b 100644 --- a/Sources/RichText/Models/Configuration.swift +++ b/Sources/RichText/Models/Configuration.swift @@ -32,7 +32,24 @@ public struct Configuration { public var errorHandler: ErrorHandler? - public var isColorsImportant: ColorPreference + /// Backing storage for ``isColorsImportant``. + /// + /// Kept separate so the framework can carry the value around without tripping its own + /// deprecation warning. + var storedColorPreference: ColorPreference + + /// The requested colour enforcement preference. + /// + /// - Warning: Assigning this does not change the generated CSS. `!important` is driven + /// entirely by ``ColorSet/isImportant`` on ``fontColor`` and ``linkColor``, and this + /// property has never been read during CSS generation. Use + /// ``RichText/colorPreference(forceColor:)``, which sets both colour sets, or pass + /// `ColorSet(light:dark:isImportant:)` directly. + @available(*, deprecated, message: "Has no effect on the generated CSS. Use .colorPreference(forceColor:) on the view, or pass ColorSet(light:dark:isImportant:) for fontColor/linkColor.") + public var isColorsImportant: ColorPreference { + get { storedColorPreference } + set { storedColorPreference = newValue } + } public var transition: Animation? @@ -52,7 +69,8 @@ public struct Configuration { /// - baseURL: Base URL for relative resources /// - mediaClickHandler: Handler for image/video click events /// - errorHandler: Handler for error events - /// - isColorsImportant: Color preference enforcement + /// - isColorsImportant: Deprecated. Recorded but never read during CSS generation; use + /// `.colorPreference(forceColor:)` or `ColorSet(light:dark:isImportant:)` instead. /// - transition: Animation for transitions public init( customCSS: String = "", @@ -93,7 +111,7 @@ public struct Configuration { self.baseURL = baseURL self.mediaClickHandler = mediaClickHandler self.errorHandler = errorHandler - self.isColorsImportant = isColorsImportant + self.storedColorPreference = isColorsImportant self.transition = transition if supportsDynamicType { diff --git a/Sources/RichText/Models/RichTextConstants.swift b/Sources/RichText/Models/RichTextConstants.swift index 3d8488a..7ff6a79 100644 --- a/Sources/RichText/Models/RichTextConstants.swift +++ b/Sources/RichText/Models/RichTextConstants.swift @@ -43,11 +43,26 @@ public struct RichTextConstants { public static let httpsScheme = "https" // MARK: - CSS Selectors and Properties (v3.0.0 - Performance optimized) - public static let imageCSS = "img{max-height: 100%%; min-height: 100%%; height:auto; max-width: 100%%; width:auto;margin-bottom:5px; border-radius: %@px; loading: lazy;}" + // Standard responsive image sizing: cap the width at the container and let the height + // follow the aspect ratio. + // + // The percentage `min-height`/`max-height` that used to sit here resolved against a + // containing block of `auto` height, so both were no-ops and neither did anything for + // the responsive behaviour. `min-height: 100%` in particular would have stretched every + // image to the full container height the moment a definite height appeared. + // + // `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 `` + // elements themselves. + public static let imageCSS = "img{height:auto; max-width: 100%%; width:auto;margin-bottom:5px; border-radius: %@px;}" 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; }" - public static let iframeCSS = "iframe{width:100%%; height:%dpx; border: none;}" + // `%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`. + public static let iframeCSS = "iframe{width:100%%; height:%ldpx; border: none;}" 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.") public static let bodyCSS = "body { margin: 0; padding: 0; -webkit-text-size-adjust: 100%; }" // MARK: - HTML5 Semantic Elements CSS (v3.0.0 - Enhanced accessibility) diff --git a/Tests/RichTextTests/RichTextSwiftTestingTests.swift b/Tests/RichTextTests/RichTextSwiftTestingTests.swift index 9386179..7de7f8c 100644 --- a/Tests/RichTextTests/RichTextSwiftTestingTests.swift +++ b/Tests/RichTextTests/RichTextSwiftTestingTests.swift @@ -26,7 +26,9 @@ struct RichTextAllTests { #expect(config.lineHeight == RichTextConstants.defaultLineHeight) #expect(config.imageRadius == RichTextConstants.defaultImageRadius) #expect(config.forceColorSchemeBackground == false) - #expect(config.isColorsImportant == .onlyLinks) + // Read through the backing storage: the public property is deprecated because it + // never reaches the generated CSS. + #expect(config.storedColorPreference == .onlyLinks) } @Test("Configuration with custom values") @@ -47,7 +49,48 @@ struct RichTextAllTests { #expect(config.lineHeight == customLineHeight) #expect(config.imageRadius == customImageRadius) #expect(config.forceColorSchemeBackground == true) - #expect(config.isColorsImportant == .all) + #expect(config.storedColorPreference == .all) + } + + @Test("Color preference is only applied through the colour sets") + func colorPreferenceOnlyAppliesThroughColorSets() { + // `isColorsImportant` is recorded but never read during CSS generation, which is + // why it is deprecated. `!important` comes from the ColorSets alone. + let viaParameter = Configuration( + fontColor: ColorSet(light: "000000", dark: "FFFFFF"), + isColorsImportant: .all + ) + #expect(!viaParameter.css(isLight: true, alignment: .leading).contains("#000000 !important")) + + let viaColorSet = Configuration( + fontColor: ColorSet(light: "000000", dark: "FFFFFF", isImportant: true) + ) + #expect(viaColorSet.css(isLight: true, alignment: .leading).contains("#000000 !important")) + } + + @Test("Generated CSS carries no dead declarations") + func generatedCSSHasNoDeadDeclarations() { + let css = Configuration().css(isLight: true, alignment: .leading) + + // `loading` is an HTML attribute, not a CSS property. `loading: lazy` in a + // stylesheet is dropped by every browser, so it only ever looked like lazy + // loading was enabled. + #expect(!css.contains("loading:")) + + // Percentage `min-height`/`max-height` on `img` resolved against a containing + // block of `auto` height, so both were no-ops. `min-height: 100%` would have + // stretched every image to the container height as soon as one appeared. + #expect(!css.contains("min-height")) + #expect(!css.contains("max-height")) + + // What actually makes images responsive. + #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. + #expect(css.contains("height:\(RichTextConstants.iframeHeight)px")) } @Test("Configuration with dynamic type support") @@ -87,12 +130,10 @@ struct RichTextAllTests { // `max-width: 100`, an invalid length that WebKit drops, and images then overflow // the web view instead of being constrained to its width. #expect(css.contains("max-width: 100%")) - #expect(css.contains("max-height: 100%")) #expect(css.contains("width:100%;")) #expect(css.contains("line-height: 150.0%")) #expect(!css.contains("max-width: 100;")) - #expect(!css.contains("max-height: 100;")) #expect(!css.contains("width:100;")) } @@ -679,8 +720,7 @@ struct RichTextAllTests { colorScheme: .auto, forceColorSchemeBackground: true, imageRadius: 8, - linkColor: ColorSet(light: "0066CC", dark: "3399FF", isImportant: true), - isColorsImportant: .all + linkColor: ColorSet(light: "0066CC", dark: "3399FF", isImportant: true) ) let richText = RichText(html: html, configuration: config)