Fix crash opening Kiosk settings in non-English locales (#4487)#4490
Conversation
There was a problem hiding this comment.
Can you use a new localized keys for the footer? Since the translation was already sent to localize (platform we use for translations) these changes won't apply after Lokalise run the GitHub workflow to sync translations.
Also, modify only the English strings
|
Wait, I have manually updated in Lokalise, I'll tag the import PR here in a bit then you can merge into your branch and see if it works |
…nt#4487) The gestureFooter call site passes the taps count as an Int, but with the format string now using %2$@ (updated in Lokalise), the SwiftGen signature is gestureFooter(_ p1: Any, _ p2: Any). Wrap the Int with String() so the CVarArg is a pointer type matching the %@ specifier. Add regression tests (KioskLocalization.test.swift) that exercise all kiosk format-string keys against every bundled locale, confirming: - Specifier count matches English - String(format:) completes without crash - Each arg appears verbatim in output Also adds a targeted test that injects each locale's gesture_footer format into LocalizedManager and calls the real L10n function path.
nstefanelli
force-pushed
the
kiosk-fix-gesture-footer-i18n-crash
branch
from
dc3adf8 to
3948efa
Compare
|
Code-wise it looks fine, let me know when you have validated it |
|
Thanks for jumping in and fixing the Lokalise translations directly — that was way faster than the workaround I had going! Rebased on top of #4493, reverted the key rename and de/nl patches since they're no longer needed. The PR now just has the call site fix (wrapping the taps count with Build passes locally, tests pass, and confirmed no crash on a German locale simulator. Ready for review whenever you get a chance! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash that occurs when opening Kiosk Mode settings in non-English locales (specifically German and Dutch) by implementing three layers of defense: (1) changing the English format string to use both placeholders as %@ with positional markers to make it crash-proof regardless of translation reordering, (2) updating German and Dutch translations to use positional markers with correct grammar, and (3) adding comprehensive regression tests to prevent future format string issues.
Changes:
- Updated the Kiosk settings footer call site to wrap the integer tap count in
String(), matching the new format string expectation - Added comprehensive regression tests (
KioskLocalization.test.swift) with two test methods: one that validates format specifier counts across all locales, and another that directly exercises the real L10n function to ensure no crashes occur - Updated the Xcode project file to include the new test file
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Tests/App/Kiosk/KioskLocalization.test.swift | New regression tests for kiosk localization, validating format strings across all bundled locales and testing the L10n function directly |
| Sources/App/Kiosk/Settings/KioskSettingsView.swift | Updated the gestureFooter call to pass the tap count as a String instead of Int to match the new format string requirements |
| HomeAssistant.xcodeproj/project.pbxproj | Added the new test file to the project build configuration |
…nt#4487) The gestureFooter call site passes the taps count as an Int, but with the format string now using %2$@ (updated in Lokalise), the SwiftGen signature is gestureFooter(_ p1: Any, _ p2: Any). Wrap the Int with String() so the CVarArg is a pointer type matching the %@ specifier. Add regression tests (KioskLocalization.test.swift) that exercise all kiosk format-string keys against every bundled locale, confirming: - Specifier count matches English - String(format:) completes without crash - Each arg appears verbatim in output Also adds a targeted test that injects each locale's gesture_footer format into LocalizedManager and calls the real L10n function path.
nstefanelli
force-pushed
the
kiosk-fix-gesture-footer-i18n-crash
branch
from
dd06b8a to
4b68710
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4490 +/- ##
=======================================
Coverage ? 42.65%
=======================================
Files ? 274
Lines ? 16230
Branches ? 0
=======================================
Hits ? 6923
Misses ? 9307
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you! |
Summary
Fixes #4487 — opening Settings → Companion App → Kiosk Mode (Labs) crashes the app immediately in German and Dutch (and potentially any other locale whose translators reordered format specifiers without positional markers).
Root cause
kiosk.security.gesture_footerinen.lprojused non-positional format specifiers:The German and Dutch translations legitimately reordered them for grammar, but without positional markers:
enTap the %@ corner %li times...deTippe **%li**-mal auf die Ecke **%@**, um...← reversednlTik **%li** keer op de hoek **%@** om...← reversedThe SwiftGen-generated
L10n.Kiosk.Security.gestureFooter(_ p1: Any, _ p2: Int)passed(String, Int)positionally toString(format: format, locale: Locale.current, arguments: args). With the reversed German/Dutch specifiers:%lireads the first slot (aStringbridged viaCVarArg) aslong int→ garbage integer (not fatal)%@reads the second slot (anIntvalue) as an Obj-C object pointer → Foundation calls-descriptionon theIntbit-pattern →EXC_BAD_ACCESSinCFStringCreateWithFormatAndArgumentsAuxThe
KioskSettingsViewfooter is rendered by default (secretExitGestureEnableddefaults totrue), so the crash triggers the moment a German/Dutch user navigates to Kiosk settings.Why this wasn't caught by CI
Tests/App/LocalizedStrings.test.swiftalready validates format specifiers match across languages, but usesNSCountedSetfor the comparison — which is order-insensitive. English{%@, %li}counted-set equals German{%li, %@}→ test passes despite the fatal reorder. This is a broader test defect that's out of scope for this PR but flagged below for follow-up.The fix (three layers of defense)
1. Swift-level crash elimination (durable, in our control)
Change the English source to use both placeholders as
%@with positional markers:SwiftGen regenerates
Strings.swiftwith signaturegestureFooter(_ p1: Any, _ p2: Any). The call site inKioskSettingsView.swiftwraps the taps count:String(viewModel.settings.secretExitGestureTaps).Why both
%@instead of%1\$@ %2\$li? Keeping both slots pointer-typed makes the function crash-proof against any future translation error. Even if a translator drops positional markers and reorders to%@ ... %@,va_argalways reads a pointer in both slots → worst case is mis-ordered text output, never a crash. The integer range is 2–5 (enforced byStepper: 2...5), so losing%li's locale-specific digit rendering has no visible impact.2. Patch
de.lprojandnl.lprojtranslationsUpdated the German and Dutch strings to use
%1\$@ %2\$@positional markers in the grammatically-correct order:These patches are transient — the nightly
fastlane update_stringscron will overwrite them with whatever Lokalise has. Layer 1 prevents any future crash regardless, but the ideal end-state requires a Lokalise source update — see the follow-up request below.3. Regression tests (
Tests/App/Kiosk/KioskLocalization.test.swift)Two new XCTest cases narrowly scoped to kiosk localization:
testKioskFormatStringsAcrossAllLocales— iterates every bundled*.lproj/Localizable.strings, enumerates known kiosk format keys (7 total), asserts specifier count matches English, invokesString(format:)with representative string args, and verifies each arg appears verbatim in the output. Catches reorder-without-positional-markers because the wrong specifier would either swallow or misrender the arg.testGestureFooterDoesNotCrashAcrossLocales— exercises the realL10n.Kiosk.Security.gestureFooterfunction path by injecting each locale's format string intoLocalizedManager, then invoking the generated function with(String, Int). If the generated signature ever regresses to a type-mismatched form, this test crashes the runner against the German/Dutch format — a loud, unmissable failure.Screenshots
N/A — the visible change is "app no longer crashes"; the footer text in German/Dutch renders the same words it always intended to, in the grammatically correct order.
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant# — not required, no user-facing behavior change
Any other notes
Follow-ups for maintainers (deliberately out of scope)
Lokalise source-of-truth update — please update the Lokalise project for `kiosk.security.gesture_footer` so the English source uses `%1$@ %2$@` and the `de`/`nl` translations are updated to use positional markers. Until this happens, the next `download_localized_strings` cron will overwrite the Layer 2 patches in this PR. Layer 1 keeps the app crash-free regardless, so there's no blocking urgency, but the de/nl text will revert to misleading word order until the Lokalise update lands.
`LocalizedStrings.test.swift` uses `NSCountedSet` (line 121) for format-specifier comparison, which is order-insensitive. This is why App crashes when enabling Kiosk Mode #4487 slipped past CI despite existing coverage. A stricter ordered comparison (with positional-marker awareness) would catch the entire class of bug across the codebase, not just kiosk. Happy to open a separate PR for this if a maintainer wants it.
Audit of other multi-arg `L10n` functions — a grep shows roughly two dozen other multi-arg `L10n.tr` call sites outside kiosk. Most take two `Any` (`%@` twice), which is safe from the pointer-dereference crash, but a few take `Int` mixed with `Any`. These warrant the same audit-and-fix treatment if the underlying translations turn out to have similar reorder-without-positional-marker issues. Out of scope for me as a kiosk contributor — flagging so a maintainer can prioritize.
Testing notes
CC
`@bgoncal` per your comment on #4487.
🤖 Draft PR — opening for early visibility; will mark ready for review after on-device verification.