Repository navigation
Fall back to default settings when page config is missing - #81
Conversation
The page HTML that carries the wgSmj* settings can be cached separately from the ext.SimpleMathJax module (CDN or reverse proxy). Right after an upgrade that renamed settings (1.0), a cached page paired with the new module left wgSmjDelimitersInlineMath null, so ensureLoaded() threw on .concat and no formula rendered. Read settings through a helper that falls back to the extension.json defaults, and test that the two stay in sync.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b3ac9fcbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function config(name) { | ||
| var value = mw.config.get(name); | ||
| return value === null || value === undefined ? defaults[name] : value; |
There was a problem hiding this comment.
Preserve the cached legacy CDN setting
On the pre-1.0 cached pages this change targets, wgSmjCdnEnabled is absent but its renamed predecessor wgSmjUseCdn is present. For installations that set the legacy value to false, this unconditional default returns true and loads MathJax from jsDelivr instead of the bundled copy; sites that disabled CDN access for CSP, privacy, or network reasons will therefore still render no formulas until the HTML cache expires. Consult wgSmjUseCdn before falling back to the default for this renamed setting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. config() now consults the pre-1.0 name for settings that were only renamed (wgSmjUseCdn → wgSmjCdnEnabled, per the "Renamed only" table in docs/mig-1.0.md) before falling back to the default. Settings whose meaning or default changed in 1.0 (wgSmjDirectMathJax etc.) are deliberately not mapped, so cached pages behave like freshly rendered ones. tests/JsDefaultsTest.php checks that each legacy mapping is listed as a rename in the guide. Browser check with a pre-1.0 page config (wgSmjUseCdn: false): MathJax is requested from extensions/SimpleMathJax/resources/MathJax/tex-chtml.js instead of jsDelivr.
There was a problem hiding this comment.
Reconsidered: 1.0 deliberately dropped pre-1.0 setting names (docs/mig-1.0.md), so the client should not read wgSmjUseCdn either. Removed in #83; only the fallback to extension.json defaults remains.
wgSmjCdnEnabled is a pure rename of wgSmjUseCdn. A cached pre-1.0 page carries only the old name, so a site with the CDN disabled would load MathJax from jsDelivr. Settings whose meaning changed in 1.0 (delimiters) are deliberately not mapped.
Falls back to extension.json defaults when the page config lacks the wgSmj* settings, e.g. HTML cached before an upgrade (jmnote/SimpleMathJax#81).
Problem
After upgrading from 0.9.x to 1.1.0, pages served from a cache populated before the upgrade (CDN or reverse proxy) still carry the old
wgSmj*names inRLCONF, whileext.SimpleMathJaxcomes from the new version.mw.config.get('wgSmjDelimitersInlineMath')is thennull,ensureLoaded()throwsCannot read properties of null (reading 'concat'), and no formula on the page renders until the cached HTML expires.Change
resources/ext.SimpleMathJax.js: read settings throughconfig(name), which falls back to adefaultsobject matchingextension.jsonwhen the page does not provide a value. All 10wgSmj*reads go through it.wgSmjEnableMenuis exported but not read by the client, so it has no client default.wgSmjUseCdn→wgSmjCdnEnabled), so a cached page from a site with the CDN disabled still loads the bundled MathJax. Settings whose meaning changed in 1.0 (delimiters) are not mapped.tests/JsDefaultsTest.php(added tomake test): everyconfig('wgSmj…')read has a default, each default equals theextension.jsonvalue, and each legacy mapping is listed as a rename indocs/mig-1.0.md.Tests
make checkspasses. The new test fails when a default is changed (checked withwgSmjScale: 2).wgSmj*removed fromRLCONFto simulate cached HTML: before, the error above and 0 formulas; after, no error and all 84 formulas rendered with MathJax 4.1.3.wgSmjUseCdn: false: MathJax is requested fromextensions/SimpleMathJax/resources/MathJax/tex-chtml.jsinstead of jsDelivr.