also looking at this
LSU's own coaching surface, and what auditing the site turned up
#12
86 files
+3943
-1190
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -258,7 +258,7 @@ Miss the next start or fail to finish a loop within the hour, and your race is o | ||
| 258 | 258 | </div> |
| 259 | 259 | </div> |
| 260 | 260 | <div class="carousel lsu-scope-home" data-carousel> |
| 261 | <div class="carousel-track lsu-scope-home" ref="lsuGallery" @scroll="lsuGalleryChanged()" @keydown.left.prevent="lsuGalleryMove(-1)" @keydown.right.prevent="lsuGalleryMove(1)" tabindex="0" data-track tabindex="0" role="group" aria-roledescription="carousel" aria-label="Season 2 Recap"> | |
| 261 | <div class="carousel-track lsu-scope-home" ref="lsuGallery" @scroll="lsuGalleryChanged()" @keydown.left.prevent="lsuGalleryMove(-1)" @keydown.right.prevent="lsuGalleryMove(1)" tabindex="0" data-track role="group" aria-roledescription="carousel" aria-label="Season 2 Recap"> | |
| 262 | 262 | <div class="carousel-slide lsu-scope-home"> |
| 263 | 263 | <img src="/assets/lsu/images/recap-01-810.webp" srcset="/assets/lsu/images/recap-01-540.webp 540w, /assets/lsu/images/recap-01-810.webp 810w, /assets/lsu/images/recap-01-1080.webp 1080w" sizes="(min-width: 1000px) 30vw, (min-width: 640px) 46vw, 100vw" alt="A runner in a dark shirt walks out of the red-lit start gantry at night." width="1080" height="1440" loading="lazy" decoding="async" class="lsu-scope-home"> |
| 264 | 264 | </div> |
| @@ -93,6 +93,40 @@ export function assetUrls(html: string): string[] { | ||
| 93 | 93 | return [...new Set(urls)] |
| 94 | 94 | } |
| 95 | 95 | |
| 96 | /** | |
| 97 | * The attribute names on one tag, in order, skipping quoted values. | |
| 98 | * | |
| 99 | * A regex over `name=` finds names inside values too - an stx binding like | |
| 100 | * `:text="a === 'x' ? b : a === 'y'"` reads as an attribute called `a` twice | |
| 101 | * - so this walks the string instead. | |
| 102 | */ | |
| 103 | export function attributeNames(attrs: string): string[] { | |
| 104 | const names: string[] = [] | |
| 105 | let i = 0 | |
| 106 | while (i < attrs.length) { | |
| 107 | while (i < attrs.length && /\s/.test(attrs[i]!)) i += 1 | |
| 108 | const start = i | |
| 109 | while (i < attrs.length && !/[\s=/>]/.test(attrs[i]!)) i += 1 | |
| 110 | const name = attrs.slice(start, i) | |
| 111 | if (!name) { i += 1; continue } | |
| 112 | names.push(name.toLowerCase()) | |
| 113 | while (i < attrs.length && /\s/.test(attrs[i]!)) i += 1 | |
| 114 | if (attrs[i] !== '=') continue | |
| 115 | i += 1 | |
| 116 | while (i < attrs.length && /\s/.test(attrs[i]!)) i += 1 | |
| 117 | const quote = attrs[i] | |
| 118 | if (quote === '"' || quote === '\'') { | |
| 119 | i += 1 | |
| 120 | while (i < attrs.length && attrs[i] !== quote) i += 1 | |
| 121 | i += 1 | |
| 122 | } | |
| 123 | else { | |
| 124 | while (i < attrs.length && !/\s/.test(attrs[i]!)) i += 1 | |
| 125 | } | |
| 126 | } | |
| 127 | return names | |
| 128 | } | |
| 129 | ||
| 96 | 130 | /** The rules, over one page. */ |
| 97 | 131 | export function auditPage(page: string, raw: string, served: Set<string>): Finding[] { |
| 98 | 132 | const findings: Finding[] = [] |
| @@ -193,7 +227,18 @@ export function auditPage(page: string, raw: string, served: Set<string>): Findi | ||
| 193 | 227 | if (!canonical || !canonical.startsWith('http')) |
| 194 | 228 | findings.push({ page, rule: 'no-canonical', detail: canonical ?? '' }) |
| 195 | 229 | |
| 196 | // 12. An asset the page names and the deploy does not carry. A stylesheet, | |
| 230 | // 12. The same attribute written twice on one tag. A parser keeps the | |
| 231 | // first and drops the rest, so the second value is silently ignored - | |
| 232 | // which is indistinguishable from it having been applied. | |
| 233 | for (const [, tag, attrs] of body.matchAll(/<([a-z][\w-]*)\b([^>]*)>/gi)) { | |
| 234 | const names = attributeNames(attrs!) | |
| 235 | for (const name of new Set(names)) { | |
| 236 | if (names.filter(other => other === name).length > 1) | |
| 237 | findings.push({ page, rule: 'duplicate-attribute', detail: `${name} on <${tag!.toLowerCase()}>` }) | |
| 238 | } | |
| 239 | } | |
| 240 | ||
| 241 | // 13. An asset the page names and the deploy does not carry. A stylesheet, | |
| 197 | 242 | // a font or a photograph that 404s is silent: the page still renders, |
| 198 | 243 | // just without it, and nothing in a build log says so. |
| 199 | 244 | for (const url of assetUrls(raw)) { |
| @@ -2,7 +2,7 @@ import { describe, expect, it } from 'bun:test' | ||
| 2 | 2 | import { readdirSync } from 'node:fs' |
| 3 | 3 | import { join, resolve } from 'node:path' |
| 4 | 4 | import { processDirectives } from '@stacksjs/stx' |
| 5 | import { assetExists, assetUrls, auditPage, definedClasses, formatFindings, redirectSources, unstyledClasses } from '../scripts/audit-html' | |
| 5 | import { assetExists, assetUrls, attributeNames, auditPage, definedClasses, formatFindings, redirectSources, unstyledClasses } from '../scripts/audit-html' | |
| 6 | 6 | |
| 7 | 7 | /** |
| 8 | 8 | * The same rules scripts/audit-html.ts runs over dist, run here over the |
| @@ -138,6 +138,21 @@ describe('each rule', () => { | ||
| 138 | 138 | expect(rules('<div style="background-image:url(/assets/lsu/images/nope.webp)"></div>')).toContain('missing-asset') |
| 139 | 139 | }) |
| 140 | 140 | |
| 141 | it('catches the same attribute written twice on one tag', () => { | |
| 142 | expect(rules('<div tabindex="0" data-track tabindex="0"></div>')).toContain('duplicate-attribute') | |
| 143 | expect(rules('<div tabindex="0" data-track></div>')).not.toContain('duplicate-attribute') | |
| 144 | }) | |
| 145 | ||
| 146 | it('does not read an attribute name out of a value', () => { | |
| 147 | // An stx binding mentions its own state names inside the quoted value. | |
| 148 | const binding = `<button :text="mode === 'new' ? a : mode === 'old' ? b : c" :disabled="busy"></button>` | |
| 149 | expect(rules(binding)).not.toContain('duplicate-attribute') | |
| 150 | ||
| 151 | expect(attributeNames(` :text="mode === 'new'" :disabled="busy" data-x`)) | |
| 152 | .toEqual([':text', ':disabled', 'data-x']) | |
| 153 | expect(attributeNames(` a=1 b='two words' c`)).toEqual(['a', 'b', 'c']) | |
| 154 | }) | |
| 155 | ||
| 141 | 156 | it('catches a page with no language', () => { |
| 142 | 157 | expect(auditPage('/t', '<html><body><h1>x</h1></body></html>', served).map(f => f.rule)).toContain('no-lang') |
| 143 | 158 | expect(rules('')).not.toContain('no-lang') |