fix(addon): make font url() rewrite quote-aware to prevent parsable-css crash - #942
Merged
Merged
Conversation
…ss crash
The build.sh rewrite that converts absolute url(/fonts/...) to relative
url(../fonts/...) only matched the unquoted form. Dev/unminified builds
preserve the source single quotes (url('/fonts/...')), so the rewrite was
a no-op and the add-on shipped absolute font URLs. Loaded as a built-in
add-on these resolve against resource://builtin-addons/fonts/... instead of
the add-on's own directory, crashing Thunderbird's static
browser_parsable_css.js check (Bug 2036665).
Capture the optional quote and re-emit it so both quoted and unquoted forms
are rewritten, and add a build-time guard that fails the build if any
absolute /fonts/ url() survives the rewrite.
Closes #941
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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?
Made the webfont
url()rewrite inpackages/addon/scripts/build.shquote-aware, and added a build-time guard.url(/fonts/...)to relativeurl(../fonts/...)previously useds{url\(/fonts/}{url(../fonts/}g, which only matched the unquoted form. It now captures an optional quote ((["\x27]?)) and re-emits it, sourl(/fonts/,url('/fonts/, andurl("/fonts/are all rewritten./fonts/url()survived, so a broken bundle can never silently ship again.I verified the new regex against the actual deployed CSS: all 36
url('/fonts/...')references convert tourl('../fonts/...'), and the guard passes on the rewritten output (and fails on the pre-fix input).AI disclosure: this change was diagnosed and written by Claude (agent), reviewed and directed by the author. The author identified the failing try run; the agent traced the root cause from the CI log, wrote the one-line regex fix and guard, and verified the rewrite against the deployed CSS.
Why?
A try push crashed in Thunderbird's static
browser_parsable_css.jscheck:The bundled CSS shipped absolute
url('/fonts/...')font references. Loaded as a built-in add-on, an absolute/fonts/...URL resolves against theresource://protocol root (resource://builtin-addons/fonts/...) rather than the add-on's own directory, so the font is missing and the static check fatally crashes (Bug 2036665).build.shalready had a rewrite step meant to prevent exactly this, but its regex only matched unquotedurl(/fonts/. Production (minified) builds strip quotes so the bug stayed hidden; the dev/unminified build that was deployed preserved the source single quotes (url('/fonts/...')), so the rewrite was a no-op and the absolute paths shipped — crashing CI.Failing run: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=51fc152ffe02dc4a5ec85cedbb188463172bb7f2&selectedTaskRun=dHZ5CqI4QRSnRU3g-hftrg.0
Limitations and Notes
.xpiwas a dev/unminified build, which is what exposed the gap. Standardizing the deploy/release path on the productionpnpm buildwould add defense in depth, though this fix makes the rewrite correct for both modes..xpirebuild + re-deploy into the comm tree and a new try push to confirm the suite goes green.Applicable Issues
Closes #941
Screenshots
N/A — build-script change.