fix img css missing % after String formatted - #76
Merged
Conversation
|
作者看到了早点修复，这里的确有bug String(format: RichTextConstants.imageCSS, "\(imageRadius)")
// 这里会冲掉imageCss中的% |
Owner
|
Thanks @diatoming, and thanks @lilin87788 for confirming. Verified the root cause: "img{max-height: 100%; min-height: 100%; ... max-width: 100%; ...}"was actually emitted as img{max-height: 100; min-height: 100; ... max-width: 100; ...}
Merging. I also added regression tests in #81 that scan every format string, so a future edit that forgets to double a |
NuPlay
added a commit
that referenced
this pull request
Aug 29, 2026
Addresses the second round of Copilot review feedback on #81. The scanner required every `%` to be followed by `%`, `@` or `d`. That pins the conversion vocabulary down as a side effect: `iframeCSS` passes a Swift `Int` to `%d`, which reads 32 bits of a 64-bit argument, and widening it to `%ld` would have been rejected by this test even though it is the more correct spelling. Test the failure mode instead. 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, so flagging the delimiters catches a missing `%%` without constraining which conversions are allowed. Verified against all seven formatted constants: they pass, the pre-#76 `imageCSS` still reports its three unescaped percent signs, and `%d`, `%ld`, `%s` and `%@` are all accepted.
NuPlay
added a commit
that referenced
this pull request
Aug 29, 2026
* test: pin down percent escaping in the formatted CSS constants Every CSS constant is rendered through `String(format:)`, which consumes a bare `%`. `imageCSS` was missing the `%%` escapes, so `max-width: 100%` was emitted as `max-width: 100` - an invalid length that WebKit discards, leaving images unconstrained by the web view width. Add two regression tests: one asserting the generated CSS still carries its percent signs, and one scanning the format strings themselves so a future edit that forgets to double a `%` fails at test time rather than in a layout bug. Requires #76. * test: scan every format string, not just three of them Addresses Copilot review feedback on #81. The input check only walked `imageCSS`, `textCSS` and `iframeCSS`, which left `linkCSS`, `cssTemplate`, `mediaCSSTemplate` and `htmlTemplate` unguarded even though they all go through `String(format:)` too. Turn the check into a parameterised test over the full set, and name the offending constant in the failure message. Also corrects the comment: not every CSS constant is formatted, only the ones carrying substitutions, and the previous wording would have misled the next person into escaping the wrong things. * test: check the shape of the bug instead of whitelisting conversions Addresses the second round of Copilot review feedback on #81. The scanner required every `%` to be followed by `%`, `@` or `d`. That pins the conversion vocabulary down as a side effect: `iframeCSS` passes a Swift `Int` to `%d`, which reads 32 bits of a 64-bit argument, and widening it to `%ld` would have been rejected by this test even though it is the more correct spelling. Test the failure mode instead. 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, so flagging the delimiters catches a missing `%%` without constraining which conversions are allowed. Verified against all seven formatted constants: they pass, the pre-#76 `imageCSS` still reports its three unescaped percent signs, and `%d`, `%ld`, `%s` and `%@` are all accepted.
This was referenced Aug 29, 2026
NuPlay
added a commit
that referenced
this pull request
Aug 29, 2026
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.
NuPlay
added a commit
that referenced
this pull request
Aug 29, 2026
…t:) (#86) 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the default image css problem.
Known Issues: