Fix/2026 09 13/audit response round1 - #41
Merged
Merged
Conversation
.material-symbols-outlined only set font-variation-settings; nothing ever told it to actually use the Material Symbols font. The Google Fonts link in +layout.svelte was loading correctly the whole time -- an earlier read of this bug (mine included) wrongly assumed a CDN-timing/FOUC race. It isn't: this is deterministic and reproduces on every load, on any connection, which is what four independent role-based audits converged on after checking the actual rule in app.css rather than guessing from the symptom. Confirmed in the compiled output, not just the source: built CSS now reads font-family:Material Symbols Outlined on the rule. Also removed docs/+page.svelte's local scoped override of the same selector -- it never set font-family either, so it was already dead (the global rule's !important on font-variation-settings dominated it), and keeping it only would have invited a future edit to 'fix' a rule with no visible effect either way. Verified: type-check clean (1507 files, 0 errors), lint clean, vitest 172/172, build succeeds, and the fix confirmed present in the actual built CSS asset -- not just inferred from source.
Each modal's Escape handler lived on the backdrop div -- a sibling, not an ancestor, of the focus-trapped content. Once focusTrap moves focus inside on open (which it always does), a keydown on the backdrop can never fire: events bubble through ancestors only. Fixed once, in focus-trap.ts itself, via an optional onEscape callback wired into the action -- every consumer inherits correct behavior from one place instead of re-implementing (and mis-wiring) it per modal. Also added role="dialog" aria-modal="true" aria-labelledby to all three modals (ConfirmationModal, SuccessModal, HistorySection's record viewer), and aria-hidden="true" on their decorative icons -- screen readers previously got no announcement that a dialog opened, or what it was called, and would speak raw icon-ligature text next to labels that already state the same thing. Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
Design-system root causes: - shadow-huge was never a defined Tailwind utility -- all four floating surfaces (2 modals, the history record panel, LanguageToggle's dropdown) silently rendered with zero elevation. Replaced with shadow-xl everywhere it was used. HistorySection's record panel additionally had no border at all as a fallback -- added one, matching the other modals. - .btn-primary was missing the border .btn-secondary carries, causing a 1px size mismatch between adjacent primary/secondary buttons. Added a transparent border of the same width so both share one box model. - Neither button class had any disabled: styling -- a disabled submit button rendered pixel-identical to an enabled one. Added disabled:opacity-50 disabled:cursor-not-allowed disabled:pointer-events-none to both, at the one shared definition instead of per call site. - Added a real .btn-compact modifier and migrated +page.svelte's three toolbar buttons onto it. They previously layered `px-3 py-1` on top of .btn-primary/.btn-secondary, hoping to override the padding -- but those classes are @apply'd in the same Tailwind layer and land later in the compiled output, so at equal specificity they always won; the override never took effect. Verified against the compiled CSS before and after. - Header's scrolled-state translucency used bg-opacity-90/dark:bg-opacity-90, both removed in Tailwind v4. Confirmed zero rule emitted for either class in the compiled output. Replaced with a computed bg-canvas/90; confirmed the real color-mix rule now compiles. Keyboard gap: - +page.svelte's dropzone (role="button") only handled Enter, not Space -- the conventional key for activating a button role, and one this same codebase's own ActionBox.svelte already gets right. Added the Space branch with preventDefault so it doesn't also scroll the page. Contrast fixes, computed from the actual hex/opacity values, not estimated: - --accent-gold (2.85:1 on light canvas) darkened to #7a6200 (5.68:1) for light mode only -- the original #b2940a already passes in dark mode (6.19:1) and is restored there. - --fg-muted (3.33:1 on light canvas -- fails even before any opacity is layered on top) darkened to #6b6b6d (5.14:1 canvas / 4.83:1 subtle). Dark mode's #8e8e93 already passed and is untouched. - .section-label, .zen-label, .zen-code-comment, Header's version string, and +page.svelte's dropzone subtitle and word-count line all stacked an opacity utility on top of an already-marginal color -- opacity alone defeats any base-color fix, so each had the opacity dropped rather than chasing it with an ever-darker base. Verified: type-check clean, lint clean, vitest 172/172, build succeeds; spot-checked the compiled CSS for shadow-xl, .btn-compact, bg-canvas/90's color-mix rule, and the corrected token values.
Docs page's left-nav section headers and right-side TOC header both used text-fg-primary/40 (~2.3:1) -- these are primary in-page navigation labels, not decoration. Its breadcrumb nav used text-fg-muted/60. Privacy page's footer date line and the report page's consent-checkbox helper text both used opacity-60/70 on top of an already-muted color. All five dropped the opacity; the first two also moved to the (now AA-passing) fg-muted token instead of fg-primary at reduced strength. Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
The generated manifest (vite.config.ts's SvelteKitPWA config, what actually ships -- it and a hand-authored static/manifest.webmanifest collide at the same build path, and the generated one wins) referenced /android-chrome-192x192.png and /android-chrome-512x512.png. Neither file exists anywhere in the repo; only web-app-manifest-192x192.png and web-app-manifest-512x512.png do, confirmed at the exact declared dimensions (192x192, 512x512) via sips. This blocks or degrades Chrome's install-to-homescreen prompt, which needs a real >=192px icon. Beyond the icon paths, the shipping config had drifted from the correct, never-served static/manifest.webmanifest in several ways: an indigo theme_color (#4338ca) that matches nothing else in the app, instead of #7a1b1b -- the actual value in app.html's own <meta name="theme-color">; a white background_color instead of #fcfbf9, this app's real --bg-canvas; a generic description instead of the real one; the name 'MonOCR Web' instead of 'MonOCR'; and literal, never-customized SvelteKit starter- template categories (['template', 'starter', 'sveltekit']) instead of this app's actual categories. Brought the shipping manifest in line with the correct content (also added the missing screenshots entry and the 96x96 icon), then deleted the now-fully-redundant static file -- it never shipped, and keeping two sources of truth is exactly what let this drift. Verified: build succeeds, and the built manifest.webmanifest inspected directly for the corrected name/description/colors/categories/icons.
og-image.jpg is 1024x1024 (verified via sips), not the 1200x630 the screenshots entry claimed -- carried over uninspected from the dead static/manifest.webmanifest in the prior commit, whose own stated verification only covered icon dimensions, not this. Caught by an independent verifier. Also dropped form_factor: 'wide' -- a square image is neither wide nor narrow, and a wrong hint is worse than none.
- Footer.svelte (rendered on every page, not scoped to one route) had two lines at opacity-60/40 on top of the inherited text-fg-secondary -- worse than several of the ones already fixed (2.66:1 and 1.85:1), and global rather than page-scoped. Missed in the prior sweep entirely. - docs/+page.svelte's own sweep was incomplete: 6 more identical-pattern labels (code-block language tags, two section h3s) still had opacity-40 on inherited text, computing to the same ~2.3:1 the already- fixed ones did. Moved to text-fg-muted (now AA-passing) instead. - contribute/+page.svelte's "OR" divider label inherited opacity-30 from its wrapping container -- meant only for the divider line, but CSS opacity cascades to all descendants, crushing the actual text to ~1.49:1. Moved the opacity onto each border line individually instead. Left alone, checked and not failing: LanguageToggle's Mon-heritage label (opacity-70 on fg-accent computes to 4.52:1, a genuine if marginal AA pass); the breadcrumb "/" and bullet-point dividers (decorative punctuation, not content); the two docs-page copy-button icons (non-text UI glyphs); Breadcrumb.svelte's own opacity-50 label (same bug, but the component is dead code, never rendered anywhere). Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
- HistorySection.svelte's 'Retry' link (real actionable text on a failed sync record) sat at opacity-60 against its row background, computing to 4.49:1 idle / 4.46:1 on hover -- just under the 4.5:1 AA line in the opposite direction from a boundary case checked earlier this branch. Removed the opacity (and the now-meaningless hover:opacity-100/ transition-opacity that existed only to compensate for it). - contribute/+page.svelte's Mon-script textarea and report/+page.svelte's corrected-text textarea both used placeholder:text-fg-muted/30 and /40 -- computing to ~1.49:1 and ~1.72:1. Same opacity-stacked-muted-text bug already fixed elsewhere on this branch, just on the placeholder: variant, which the earlier greps for plain opacity-NN and non-placeholder text-*/NN patterns didn't catch. Also corrects a false claim in the immediately prior commit's message: it said contribute's OR-divider had 'an existing asymmetry' between its two border lines. Checked full file history this time -- there wasn't one; both lines always shared one wrapping div's opacity equally. The fix itself (splitting the opacity onto each line individually) was still correct, only that one sentence describing prior state was wrong. Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
Removed the Mon-heritage subtitle under the language name in LanguageToggle's dropdown (all three locales), per direct request. The message key (nav_mon_heritage) is now unreferenced anywhere in src, so removed it from all three locale files too, and regenerated paraglide's compiled output to match. Left about_vulnerable untouched -- a distinct, still-used message describing the UNESCO classification on the About content, not this dropdown label. Verified: type-check clean, lint clean (also dropped LanguageToggles now-unused m import), vitest 172/172, build succeeds.
Removed the hardcoded contributors list (3 names, roles, and external profile links) from the docs page, along with its left-nav/TOC entries and the two message keys that became unreferenced as a result (docs_governance_contributors, docs_contributors_title), then regenerated paraglide to match. Footer prev/next navigation is hardcoded to fixed routes, not derived from the nav arrays, so removing this entry does not affect it. Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
Section 7 (Global Compliance) had **GDPR**/**CCPA** typed as literal Markdown-style asterisks directly in the Svelte template rather than <strong> tags -- this repo has no Markdown rendering pipeline, so users saw the raw asterisks on screen, on a legal/compliance page. Flagged by an earlier audit, not actually fixed until now -- confirmed live via Playwright against the dev server at /privacy (rendered body text contained a literal '**' before the fix, does not after). Also confirmed, while checking: the page's two <footer> elements (the shared layout Footer and this page's own effective-date footer, nested inside <main>) are not an accessibility bug -- only one is exposed as the contentinfo landmark, which is correct per the HTML5 spec (a footer nested in a sectioning root gets no implicit landmark role). Verified: type-check clean, lint clean, vitest 172/172, build succeeds, and the live dev server re-checked directly.
Footer read 'MonOCR Version 0.2.0' as a literal string; the real package.json version is 0.4.0 -- two minors stale. Header.svelte already solved this exact problem with a vite __APP_VERSION__ define injected from package.json at build time (itself fixing a prior '1.0.0 vs 0.3.0' drift, per its own comment); this page just never got the same fix. Verified: type-check clean, lint clean, vitest 172/172, build succeeds.
Every icon in the app now renders through one Icon.svelte component
(~20 call sites migrated) instead of a ligature web font loaded from a
Google Fonts CDN. This is the root architectural cause behind the
literal-icon-name-as-text production bug fixed earlier this branch --
the ligature approach has no fallback when the font is slow, blocked, or
(as it turned out) missing font-family entirely; an inline SVG has no
font/network dependency and no text-fallback failure mode. Matches the
pattern this app already used correctly for the header hamburger, theme
toggle, and language toggle icons -- and the direction multiple design
audits converged on independently.
Icon names keep each old Material Symbols identifier (upload_file,
check_circle, etc.) as their lookup key, so Badge.svelte's icon prop and
every dynamic {fileType.includes('pdf') ? ... : ...} call site needed no
rewiring beyond the wrapper element.
Removed as dead weight once the migration was complete:
- The Google Fonts <link> for Material Symbols, and the CSS rule that
loaded it (+layout.svelte, app.css) -- along with the unused
.animate-spin-slow keyframe sitting next to it.
- A second Google Fonts <link> for Public Sans and its --font-public-sans
token, confirmed applied nowhere in the codebase (same dead-font
pattern already found and fixed on the sibling portfolio.zinmin.2025
site).
- Three fully-dead components (Button.svelte, Breadcrumb.svelte,
Dropzone.svelte) -- each exported from the barrel, none imported
anywhere; every real call site already hand-rolled its own equivalent
inline instead (and had already drifted from what the shared
component did).
Also, while migrating docs/+page.svelte's copy-code buttons: fixed their
hover-only visibility (invisible and undiscoverable on touch devices) and
32px hit area (under the 44px minimum), and added the same copy affordance
to the CLI reference block, which had none at all despite being the
section most worth copying from.
Verified: type-check clean (0 errors), lint clean, vitest 172/172, build
succeeds -- and live-checked with Playwright against the built preview
server: icons render as real SVG glyphs (not text), zero Google Fonts
requests fire, zero console errors.
Migrated the two remaining material-symbols-outlined spans to Icon. Separately, per direct request: the 104px gap between the Error Type pills and the consent checkbox (mb-16 + pt-10) read as dead space -- tightened to mb-16 + pt-6. The Share Correction / Cancel Feedback pair was two equally-heavy full-width buttons stacked vertically; Cancel is now a lighter text-only action beside the primary button instead of competing with it for visual weight, matching this session's compact, GitHub-inspired direction rather than the site's original mega-button pattern. Verified: type-check clean, lint clean, vitest 172/172, build succeeds, and visually re-checked via Playwright screenshot.
Per direct request, and applied consistently across the whole app, not just the one screenshotted instance: GitHub's own buttons size to their content, they don't stretch to fill their container. HistorySection's own confirm dialog already did this correctly (flex justify-end + min-w-[100px]) -- every other stretched button in the app was the exception, not the rule. - ConfirmationModal: Cancel/Confirm were flex-1 (50/50 split); now right-aligned, content-sized, matching HistorySection's own dialog. - SuccessModal: the solitary Done button was w-full; now centered and content-sized (the modal's own content is text-center, so this reads as one deliberate choice, not a stray override). - report/+page.svelte: Share Correction was flex-1, forcing it to fill whatever space Cancel didn't take. Now min-w-[140px], sized to its own content next to Cancel, not to the row. - contribute/+page.svelte: Submit Contribution was w-full inside a centered max-w-md wrapper; now min-w-[160px], left-aligned with the rest of the page's content instead of centered on its own. Verified: type-check clean, lint clean, vitest 172/172, build succeeds, and visually re-checked both pages via Playwright screenshot.
Found by the verifier checking the previous button-sizing commit: ConfirmationModal's confirm button carried border-red-500/bg-red-500 as plain utility classes on top of .btn-primary, but .btn-primary's own @apply'd background is in the same Tailwind layer and wins the cascade regardless of markup order -- the same issue already documented for .btn-compact. The button rendered near-black, undercutting the one visual signal that a confirm action (clearing all history) is irreversible. Fixed with a real .btn-danger modifier, defined after .btn-primary in the same layer so it wins instead of losing -- confirmed in the compiled CSS (byte offset after .btn-primary) and by computed style in a live browser (renders Tailwind's actual red-500, not black). Also restored a responsive fallback the button-sizing fix had dropped: the footer was flex-col+sm:flex-row (stacks full-width on narrow screens, right-aligned content-sized on wider ones); the sizing fix had collapsed it to always-row. Full width only below the sm breakpoint, content-sized above it -- keeps a headroom margin for whatever a future label or locale needs, per the verifier's note. Verified: type-check clean, lint clean, vitest 172/172, build succeeds; compiled-CSS cascade order and live computed background-color both directly confirmed, not inferred.
New file -- this repo had no prior benchmark.md. Covers apps/web's 16 commits (icon-rendering root cause, the full SVG icon migration, modal accessibility, four rounds of contrast fixes, two rounds of PWA manifest fixes, content-scope removals, content-accuracy fixes, button consistency, a destructive-button color fix), the verification discipline behind them (7 adversarial passes, 4 of which found something real), explicit known gaps not yet acted on, and the fresh apps/android and apps/ios audits (2 platforms, real blocking findings each, no fixes applied yet).
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
1 participant
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.
What changed
Why
How it was verified
pnpm --filter ./apps/web testpnpm --filter ./apps/web run lintcd services/feedback && go vet ./... && go test ./...(if Go changed)cd apps/android && ./gradlew testDebugUnitTest(needs
JAVA_HOMEon Android Studio's JBR; see apps/android/README.md).CI does not run them, so this is the only gate on that suite.
MonOcrCoretests, if iOS changed:cd apps/ios && sh Scripts/swift-test.sh.The
ios-corejob runs these on every push, so this is a faster copy of agate that does exist. The app target is still not built anywhere.
Claims