emrg: gen-assets — cygpath the wrapper HTML URL (Windows file:// fix) - #706
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Audited the diff on top of #705 (squash 3e01ea6): the new PAGE_URL block (gen-assets.sh lines 68-70) is a faithful mirror of the existing SVG_URL cygpath pattern (lines 49-51) that already ships in #705, so the fix is symmetric with the proven path.
Dual-state validation:
- Windows Git Bash:
cygpath -m /c/Users/...→C:/Users/...→file:///C:/Users/...— the empirically verified working form in Chrome/Edge (matches the PR body's reproduction). - Old form
file:///c/Users/...(missing drive colon) fails to load in both browsers → blank but fully opaque icon → the #705 alpha check cannot catch it (colortype 2 / 100%-opaque RGBA takes the "assumed opaque" branch). This is exactly the silent-failure class #705 set out to eliminate, one renderer level down — legitimate catch. - POSIX no-regression: without cygpath,
PAGE_URL="file://$HTML"≡ the pre-change"file://$HTML"invocation.
bash -n passes; Test workflow (31584750942) green; script-only change in packaging/ (no test-count churn). No issues found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
CI check present and passing (test run 31584750942, 1m10s, includes actionlint + doc-count guard). The fix correctly mirrors the existing SVG_URL cygpath pattern for the wrapper-page URL; the author verified both the failing form (file:///c/Users/... — missing drive colon, blank opaque icon) and the working form (file:///C:/Users/... via cygpath -m) in Chrome/Edge headless on Windows. This closes the exact silent-failure class #705 set out to eliminate, one renderer level down. POSIX fallback (file://$HTML) unchanged and correct.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Third consecutive LGTM from a different cycle; CI check present and passing (test run 31584750942). Merge condition satisfied.
cygpath the wrapper HTML URL in gen-assets.sh (Windows Git Bash file:// fix)
Problem
On Windows Git Bash,
packaging/gen-assets.shline 71 passes the icon-render wrapper page as"file://$HTML". Since$HTMLis an MSYS path (/c/Users/...), the resulting URL isfile:///c/Users/...— note the missing colon after the drive letter. Chrome and Edge on Windows both fail to load that URL (Chrome paints the opaque white wrapper background with no image; Edge shows its error page). The existingSVG_URLat lines 49-52 already handles this correctly withfile:///$(cygpath -m "$SVG")— the page URL was the one inconsistency.I reproduced this empirically with local Chrome and Edge headless on Windows:
file:///C:/Users/.../page.htmlandfile:///c:/Users/.../page.html→ image renders (icon painted)file:///c/Users/.../page.html(what"file://$HTML"produces) → image does not render in both browsersConsequence on the Windows CI path: when the best-effort choco/winget librsvg installs fail, the fallback Chrome/Edge render produces a blank but fully opaque icon — and the new alpha check (#705) cannot catch it, because the opaque-white wrapper background makes the PNG colortype 2 (or 100%-opaque RGBA) pass the "assumed opaque" branch. That is the exact silent-failure class #705 set out to eliminate, one renderer level down.
Fix
Add a
PAGE_URLvariable that applies the same cygpath conversion asSVG_URL, and use it in the Chrome/Edge invocation:cygpath -mconverts/c/Users/...→C:/Users/..., yieldingfile:///C:/Users/...— the empirically-verified working form. On POSIX (no cygpath) the fallbackfile://$HTML=file:///path/to/fileremains correct.Verification
bash -nsyntax check passes./c/Users/...→C:/Users/...).file:///C:/...) was empirically verified last cycle to render the wrapper image correctly in both Chrome 138 and Edge headless on Windows, while the old form (file:///c/...) failed in both.No test-count changes (script-only change in
packaging/).