Repository navigation
Views own markup: every view's JS/CSS moves to a registered asset (TIGER-384) - #368
Merged
Merged
Conversation
…ER-384)
A `.phtml` owns markup; behavior is a JS asset and styling is the skin's job. The
`pageScript()`/`pageStyle()` seam for that was built and tested, the rule was in the
helper docblocks — and the tree still carried ~5,400 lines of inline JavaScript across
47 views, because nothing failed when someone typed `<script>`.
52 views converted: each inline block becomes a real asset file, each hand-written
`<script src>` / `<link rel="stylesheet">` becomes a registration. Server values that
used to be interpolated into the blob now ride a `data-*` attribute, and the localized
strings go through the `$this->i18n()` carrier (`Tiger.t`) — so a page ships only the
strings its own JS uses, as values, never the key taxonomy.
Found and fixed on the way: **a registration from inside `partial()` was silently
discarded.** `Zend_View_Helper_Partial` renders against a `clone` of the view and ZF1
binds a helper to a view only the first time THAT view instantiates it, so a helper
first used inside a partial wrote the slot on a throwaway clone. `profile.sms.js` has
been registered that way since it shipped and never loaded. The slot is now written on
every distinct view in play, with static dedupe so the pair can't double-emit.
The rule is now enforced rather than documented:
- AGENTS.md gains the section the helper docblocks already pointed at;
- tests/Unit/View/ViewOwnsMarkupTest.php fails CI on a `<script>`/`<style>`/
stylesheet-`<link>` in any view, and mutation-tests its own matcher;
- the CI javascript job syntax-checks EVERY shipped browser asset, not just the
theme's — a parse error in a module asset kills a screen silently behind a 200;
- paradigm-scan stops reporting a `<script>` that lives inside PHP code or a comment
(three false reports against cms/page/design), exempts a view that IS the document,
and follows `pageScript()` when deciding whether a form has an AJAX handler.
Two structural exemptions stay, neither a preference: an email template (mail clients
drop `<link>`) and a view whose controller disabled the layout, which is a layout
wearing a view's filename and has no slot to register into.
Scan: 108 views, 0 findings on all four checks. Suite: 2753 tests green.
…xempt JSON-LD Four views registered a library the admin layout loads unconditionally on every admin page (tiger-upload, Sortable, tiger.media-picker) — duplicate <script src> tags before this change and duplicate registrations after it. Dropped; the layout is the one loader. tiger.validate.js / tiger.password-strength.js stay registered by the profile Security pane, because only the AUTH layout loads those. A JSON-LD `<script type="application/ld+json">` is now exempt from the rule in both the guard and the scan: it carries DATA, not behavior, and the spec requires it inline. Also corrected auth-probe's un-gate advice, which told the operator to swap dev to Google's always-pass test keys — TIGER-385 established those are reCAPTCHA v2 only, so no configuration un-gates a v3 install. The checks belong to a headless browser run; verified by hand in a real browser on dev: a failed login returns 401 with the generic message, and two unknown identifiers plus a real user with a wrong password all produce an identical refusal.
Unrelated to the view work, but it failed a leg of this PR's first run and passed on the retry, which is how a flake earns a fix instead of a shrug. `hexdec($hex) / 1000` returns an INT when PHP divides exactly, so a v7 timestamp with no millisecond remainder came back as an int and broke the declared `@return float` — about one run in a thousand. `UuidTest::timeof_v7_is_close_to_now` is probabilistic and only catches it on that run; added a deterministic case pinning an exact second, and mutation-verified it (revert the cast and it fails with the identical CI message).
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
A
.phtmlowns markup; behavior is a JS asset and styling is the skin's job. ThepageScript()/pageStyle()seam for that was already built and unit-tested, and the rule was inthe helper docblocks — and the tree still carried ~5,400 lines of inline JavaScript across 47
views, because nothing failed when someone typed
<script>.52 views converted. Each inline block becomes a real asset file; each hand-written
<script src>/<link rel="stylesheet">becomes a registration. Server values that used to beinterpolated into the blob now ride a
data-*attribute, and localized strings go through the$this->i18n()carrier (Tiger.t) — so a page ships only the strings its own JS uses, as values,never the key taxonomy.
Scan is clean on all four checks: 108 views, 0 findings (was 75 across 53 files once the
detector stopped missing
<script src>).The bug this turned up
A registration from inside
partial()was silently discarded.Zend_View_Helper_Partialrenders against a
cloneof the view, and ZF1 binds a helper to a view only the first time thatview instantiates it — so a helper first used inside a partial wrote the slot on a throwaway clone.
profile.sms.jshas been registered that way since it shipped and has never loaded: the Phone& SMS card on
/profilehad no behavior at all, behind a 200. Proven before and after on dev.The slot is now written on every distinct view in play (the bound one and the MVC view the layout
renders against), with static dedupe so the pair can't double-emit. The channel is still the
pageScripts/pageHeadview var, so every theme layout that already echoes those keepsworking with no change — including themes shipped outside this repo.
The rule is now enforced, not documented
tests/Unit/View/ViewOwnsMarkupTest.phpfails CI on a<script>/<style>/stylesheet-<link>in any view, and mutation-tests its own matcher — a validator that only ever passes proves nothing.
javascriptjob now syntax-checks every shipped browser asset (84 files), not justthe theme's. A parse error in a module asset kills a screen silently behind a 200, which is
exactly how the three TigerImage studio bugs shipped green.
<script>that lives inside PHP code or a comment (threefalse reports against
cms/page/design.phtml, which emits exactly one), exempts a view that isthe document, and follows
pageScript()when deciding whether a form has an AJAX handler.Three structural exemptions, none a matter of taste: an email template (mail clients drop
<link>),a view whose controller disabled the layout (a layout wearing a view's filename — no slot to
register into), and a JSON-LD block (data, not behavior, and inline by spec).
Deploy note
Thirteen modules get an
assets/directory for the first time (access,agent,ally,backup,blog,cms,code,identity,mcp,media,register,signup,translations), so theyneed their
public/_modules/<slug>link published.Tiger_Module_Installer::publishAllAssets()already runs from
tiger migrate,Tiger_Update_Coreandlink:assets, so a normal update coversit — verified by running
link:assetson dev, which created all thirteen.Verification
node --checkclean.(
/admin, all of/access,/cms,/code,/media,/system/*,/translations,/backup,/agent/*,/mcp/admin,/blog,/ally,/identity,/analytics,/profile,/profile/org,/auth/*,/signup)./_tiger/js/*,/_modules/*/js/*,/_theme/js/*).tiger.auth-login.js; the dashboard'sMuuri grid lays out and fades in from the extracted CSS+JS;
/profileloads all five paneassets including
profile.sms.jsand renders contact/address cards from thedata-*payloads; the forgot-password flow completes and returns the non-enumerating message; the
Media library fetches its grid over
/api; the live reset link registers its JS while thedead-link state correctly registers none.
on four editors; Sortable → menu builder; lightbox/muuri → modules/add).
Those three were then run by hand in a real browser, where they pass: a failed login returns 401
with the generic message, and two unknown identifiers plus a real user with a wrong password all
produce an identical refusal.
Out of scope, filed separately
The remaining inline tags on a rendered page come from PHP, not views: dashboard widgets that
build HTML strings (
Register_Widget_Registration, and TigerShield's widget from its own repo) andTiger_View_Helper_FormRecaptcha's v3 bind script. Same rule, different surface — and the captchapath in particular deserves its own verification pass rather than riding along in a 100-file
refactor.