From 9e25ae90254e3f43f3359a99bf380e1ee5dfb39a Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:04:50 +1000 Subject: [PATCH 1/5] Make the Android store look and read like the rest of the app Apps without a banner or screenshot show their icon on a blurred wash of itself, or a VR or app glyph, rather than a giant letter. Every card says whether it will run on the Frame (works, should install, not checked yet, won't run), in place of "Free" on every card. The featured app isn't repeated in the rows below, and no row repeats another. Its buttons stay in the app: Install on the Frame opens the details with the install under way, and Details opens them. It's chosen from apps that can run, with real artwork, VR first. The stray "Browse SideQuest" link goes; the Sources dialog lists SideQuest with a link to its website instead of a switch that did nothing. The "All sources" drop-down goes too, as the Sources dialog already chooses them. The copy is plain and British ("Apps and games for the Frame", no "Find your next favorite"), and Games and Android point at each other. Two functions were each declared twice across the page's scripts, so the later one silently replaced the earlier everywhere: the store's details dialog opened empty (the Devices tab's renderDetail ran instead), and Home's Desktop view never found the headset's panels (the panel switcher's loader ran instead). They're renamed, and a test fails on any top-level name declared twice. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/sidequest.md | 2 + tests/test_page_scripts.py | 22 ++++++ ui/index.html | 137 +++++++++++++++++++++++++------------ 3 files changed, 117 insertions(+), 44 deletions(-) create mode 100644 tests/test_page_scripts.py diff --git a/docs/sidequest.md b/docs/sidequest.md index eba0f02..23311d0 100644 --- a/docs/sidequest.md +++ b/docs/sidequest.md @@ -65,6 +65,8 @@ flow, not evidence of an anonymous download API. It is not implemented here. - `search()` raises a user-readable `SourceError` with the browse URL (zero limit returns no rows). It does not invent app results or report a false “no matching games”. The aggregate search UI should surface this source error. + In the app, Discover doesn't link to it; the **Sources** dialog lists SideQuest + with a link to its website instead of an on/off switch. - `details()` accepts a numeric listing id and returns its canonical page link, `downloadable: False`, empty versions/tags/headsets and the `images` shape `{icon: None, banner: None, screenshots: []}`. Name is explicitly a listing id; diff --git a/tests/test_page_scripts.py b/tests/test_page_scripts.py new file mode 100644 index 0000000..35ba048 --- /dev/null +++ b/tests/test_page_scripts.py @@ -0,0 +1,22 @@ +"""The page's inline scripts share one global scope, so a second top-level function or +variable with a name already used replaces the first everywhere, silently.""" +import collections +import pathlib +import re +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[1] + + +class PageScripts(unittest.TestCase): + def test_no_top_level_name_is_declared_twice(self): + page = (ROOT / 'ui/index.html').read_text(encoding='utf-8') + names = collections.Counter() + for js in re.findall(r'', page, re.S): + for m in re.finditer(r'^(?:async\s+)?function\s+(\w+)|^(?:const|let|var)\s+(\w+)\s*=', js, re.M): + names[m.group(1) or m.group(2)] += 1 + self.assertEqual({k: n for k, n in names.items() if n > 1}, {}) + + +if __name__ == '__main__': + unittest.main() diff --git a/ui/index.html b/ui/index.html index 59a80c6..112f99c 100644 --- a/ui/index.html +++ b/ui/index.html @@ -569,6 +569,8 @@ .store-chips button[aria-pressed=true] { background:#235779; border-color:#5287a5; color:white; } .store-source-select select { max-width:180px; padding:8px 26px 8px 10px; font:inherit; font-size:12px; border:0; border-radius:5px; color:var(--text); background:#1d2b3b; } .store-notice { color:#a8b5c2; font-size:12px; margin:12px 0; } + .store-heading a, .cross-link a { color:var(--link); } + .cross-link { margin:-6px 0 12px; } body:has(.page.on #appStore:not([hidden])) #offline { padding:10px 16px; background:#ffffff06; box-shadow:inset 3px 0 0 var(--dim); } body:has(.page.on #appStore:not([hidden])) #offline .s { display:none; } .store-result-count { font-size:13px; color:var(--muted); margin-bottom:14px; } @@ -576,14 +578,24 @@ .store-hero { position:relative; display:block; width:100%; height:270px; border:0; padding:0; border-radius:12px; overflow:hidden; text-align:left; margin:0 0 32px; isolation:isolate; } .store-art { position:relative; display:block; overflow:hidden; background:radial-gradient(ellipse at 75% 10%, hsl(var(--hue) 50% 40%),transparent 70%),linear-gradient(140deg,hsl(var(--hue) 35% 18%),#101827); } .store-art > img { position:absolute; inset:0; width:100%; height:100%; object-fit:cover; z-index:1; } - .art-initial { position:absolute; inset:0; display:flex; align-items:center; justify-content:center; font-size:70px; font-weight:700; color:#ffffff45; letter-spacing:-4px; } + .store-art > img.art-wash { inset:-24px; width:calc(100% + 48px); height:calc(100% + 48px); z-index:0; filter:blur(26px) saturate(1.4); opacity:.55; } + .store-art > img.art-icon { inset:0; margin:auto; width:64px; height:64px; border-radius:15px; z-index:0; box-shadow:0 10px 28px #0007; } + .detail-banner > img.art-icon, .store-hero .store-art > img.art-icon { width:88px; height:88px; border-radius:20px; } + .store-art .art-glyph { position:absolute; inset:0; display:grid; place-items:center; color:hsl(var(--hue) 45% 72% / .38); } + .store-art .art-glyph svg { width:34%; max-width:84px; height:auto; } + .runs { font-size:11px; font-weight:500; padding:3px 8px; border-radius:10px; white-space:nowrap; } + .runs.works { color:#c6efb0; background:#70bb5426; } .runs.ready { color:#b7d8ec; background:#6ec9ff1a; } + .runs.unknown { color:#a8b5c2; background:#ffffff0d; } .runs.blocked { color:#f1c28a; background:#d9a23a22; } .store-hero > .store-art { position:absolute; inset:0; } .store-hero::after { content:""; position:absolute; inset:0; background:linear-gradient(90deg,#081421ec 0%,#08142175 48%,#08142108 80%),linear-gradient(0deg,#08142170,transparent); z-index:2; } .hero-copy { position:relative; z-index:3; padding:30px; max-width:550px; display:flex; align-items:flex-start; flex-direction:column; gap:12px; } .hero-copy .store-eyebrow { color:#c2e7ff; } .hero-copy strong { color:white; font-size:38px; letter-spacing:-.8px; line-height:1.1; } - .hero-copy > span:not(.store-eyebrow):not(.hero-cta) { color:#e0e8ef; font-size:16px; font-weight:400; max-width:380px; line-height:1.6; } - .hero-cta { color:white; display:inline-flex; align-items:center; background:#ffffff20; border:1px solid #ffffff40; border-radius:6px; padding:10px 18px; font-size:13px; margin-top:6px; } + .hero-copy .hero-sum { color:#e0e8ef; font-size:16px; font-weight:400; max-width:380px; line-height:1.6; } + .hero-actions { display:flex; gap:10px; margin-top:6px; } + .hero-actions button { height:38px; border-radius:6px; padding:0 18px; } + .hero-details { background:#ffffff20; color:#fff; border:1px solid #ffffff40; } + .store-hero { cursor:pointer; } .store-row { margin-bottom:34px; } .store-row-head { display:flex; align-items:baseline; gap:12px; margin-bottom:14px; } .store-row-head h2 { text-transform:none; letter-spacing:-.3px; font-size:21px; font-weight:600; color:var(--bright); margin:0; } @@ -659,6 +671,7 @@ .source-setting strong { display:block; font-size:14px; color:white; } .source-setting p { color:#a3b4c3; font-size:12px; margin:3px 0; } .source-setting small { color:var(--muted); font-size:10px; } + .source-web { font-size:12px; white-space:nowrap; } .source-toggle { appearance:none; width:38px; height:23px; flex:none; border-radius:15px; background:#465463; position:relative; cursor:pointer; margin:0; } .source-toggle::after { content:""; position:absolute; top:3px; left:3px; width:17px; height:17px; background:#fff; border-radius:50%; transition:transform .15s; } .source-toggle:checked { background:#208dca; } @@ -682,7 +695,7 @@ .store-chips { gap:5px; } .store-chips button { font-size:11px; padding:0 10px; } .store-hero { height:285px; border-radius:9px; margin-bottom:26px; } .hero-copy { padding:26px 22px; } .hero-copy strong { font-size:29px; } - .hero-copy > span:not(.store-eyebrow):not(.hero-cta) { font-size:13px; max-width:260px; } + .hero-copy .hero-sum { font-size:13px; max-width:260px; } .store-row-head { display:block; } .store-row-head h2 { font-size:18px; } .store-row-head p { margin-top:3px; } .store-card-content { padding:10px; } .store-card-title { gap:6px; } .store-card-title strong { font-size:13px; } .store-card-title .store-icon { width:25px; height:25px; font-size:12px; border-radius:6px; } @@ -710,7 +723,7 @@ @@ -964,6 +977,8 @@

Get games

+

Your Steam library and the Steam store. Free VR games and apps from outside Steam are on + Android.

@@ -986,11 +1001,11 @@
-
+
-
A little more possibility

Find your next favorite.

Great games and useful apps. All in one place, all free to explore.

+

Apps and games for the Frame

Free Android apps and VR games from open app stores and developers' own pages, each marked with whether it runs on the Frame. Steam games are in Games.

-
+
@@ -1260,9 +1275,9 @@ -
Your app shelves

A world of apps.

Choose where you discover games and apps. We bring them together for you.

+

Sources

Where Discover looks for apps. Turn a source off to hide its apps.

-

Add a source

Have a favorite F-Droid repository? Add its address to browse its apps here.

Verify this source (optional)
+

Add a source

Add an F-Droid repository by its address to browse its apps here.

Verify this source (optional)

 @@ -3158,46 +3173,80 @@ loadOwned(); const sourceState = { vr: null, installable: false, source: "", apps: [], sources: [], request: 0, detail: null, detailRequest: 0, jobs: new Map() }; const storeName = s => String(s || "").replace(/ \(demo\)$/i, ""); const storeHue = name => [...String(name)].reduce((n, c) => (n * 31 + c.charCodeAt(0)) >>> 0, 7) % 360; -const storeInitial = name => [...String(name || "?")][0].toUpperCase(); +const storeInitial = name => [...String(name || "?")][0].toUpperCase(); // the small icon's stand-in const storeText = text => String(text || "").replace(/<[^>]*>/g, " ").replace(/ /g, " "); const storeOpenSource = o => o.open_source === true || /^(MIT|Apache|[AL]?GPL|BSD|MPL|ISC|CC0|Unlicense)/i.test(o.license || ""); function sourcePage(url) { try { const u = new URL(url); return ["https:", "http:"].includes(u.protocol) ? u.href : ""; } catch { return ""; } } function storeImage(path, alt, eager=false) { return /^\/source-image\/[\w-]+$/.test(path || "") ? `${esc(alt)}` : ""; } +// Artwork: the banner or first screenshot. Under it (and instead of it, when there's none, or it +// doesn't load) the app's icon on a blurred wash of itself, or failing that a VR or app glyph. +const GLYPH_VR = ``; +const GLYPH_APP = ``; function storeArt(o, cls="", eager=false, path=null, alt=null) { const image = path || o.artwork?.banner || o.artwork?.screenshots?.[0]; - return `${storeImage(image, alt || `${o.name} artwork`, eager)}`; + const icon = !path && /^\/source-image\/[\w-]+$/.test(o.artwork?.icon || "") ? o.artwork.icon : ""; + const under = path ? "" : icon + ? `` + : `${o.vr ? GLYPH_VR : GLYPH_APP}`; + return `${under}${storeImage(image, alt || `${o.name} artwork`, eager)}`; } function storeIcon(o) { return ``; } -document.addEventListener("error", e => { if (e.target.matches?.(".store-art img, .store-icon img")) e.target.remove(); }, true); +document.addEventListener("error", e => { + if (!e.target.matches?.(".store-art img, .store-icon img")) return; + const art = e.target.closest(".store-art"); + e.target.remove(); + // Nothing left to show: the glyph rather than an empty wash. + if (art && !art.querySelector("img") && !art.querySelector(".art-glyph")) art.insertAdjacentHTML("afterbegin", `${GLYPH_APP}`); +}, true); function sourceMark(s) { const name=storeName(s); return ``; } -function sourceDescription(s) { return s.description || ({fdroid:"Independent, open-source apps",sidequest:"Discover something new in VR",itch:"Games from independent creators",github:"Direct from the people who make it"})[s.kind] || (s.name.includes("GitHub") ? "Direct from the people who make it" : s.name.includes("Droid") ? "Independent, open-source apps" : s.name.includes("SideQuest") ? "Discover something new in VR" : s.name.includes("itch") ? "Games from independent creators" : "Apps shared by this community"); } +function sourceDescription(s) { + const kinds = {fdroid:"Free and open-source Android apps", itch:"Games from independent developers", github:"Releases straight from the developers", + sidequest:"VR games and apps. Its terms don't allow searching it from here, so browse it on its website."}; + return s.description || kinds[s.kind] || "Apps from this repository"; +} async function loadSources() { try { const {sources} = await api("/api/sources"); sourceState.sources = sources; - $("sourceFilter").innerHTML = `` + sources.filter(s=>s.enabled).map(s=>``).join(""); - $("sourceFilter").value = sourceState.source; - $("sourcesList").innerHTML = sources.map(s=>`
${sourceMark(s.name)}
${esc(storeName(s.name))}

${esc(sourceDescription(s))}

${esc(({official:"Official store",community:"Community",user:"Added by you"})[s.trust] || "Community")}${s.builtin === false ? `` : ""}
`).join("") || `

Your sources will appear here when they are available.

`; + $("sourcesList").innerHTML = sources.map(s=>`
${sourceMark(s.name)}
${esc(storeName(s.name))}

${esc(sourceDescription(s))}

${esc(({official:"Official store",community:"Community",user:"Added by you"})[s.trust] || "Community")}${s.builtin === false ? `` : ""}
${s.page_only ? `Website ↗` : ``}
`).join("") || `

Your sources will appear here when they are available.

`; } catch (e) { $("sourceError").textContent = e.message; } } +// Will it run? Short enough for a card; the full sentence is in its tooltip and the details. +const RUNS = { works: ["✓ Works on the Frame", "works"], ready: ["Should install", "ready"], unknown: ["Not checked yet", "unknown"], blocked: ["Won't run", "blocked"] }; +function storeRuns(o) { + const [text, cls] = RUNS[o.verdict?.tone] || RUNS.unknown; + return `${text}`; +} +const storeKind = o => o.vr ? "VR" : o.vr === false ? "Flat-screen app" : "Android app"; function storeCard(app) { const i=sourceState.apps.indexOf(app), o=app.offers[0]; - return ``; + return ``; } -function storeRow(title, subtitle, apps) { return apps.length ? `

${esc(title)}

${esc(subtitle)}

${apps.slice(0,5).map(storeCard).join("")}
` : ""; } -function storeEmpty() { - return `

A little further to explore.

No apps found this time. Try a different name, clear your filters, or explore another source.

`; +function storeRow(title, subtitle, apps) { return apps.length ? `

${esc(title)}

${subtitle ? `

${esc(subtitle)}

` : ""}
${apps.slice(0,5).map(storeCard).join("")}
` : ""; } +function storeEmpty(message) { + return `

${message ? "Couldn't load apps" : "No apps found"}

${message ? esc(message) : "Try another name, or clear the filters."}

`; } function renderStore() { const apps=sourceState.apps, browsing=!$("sourceQ").value.trim() && sourceState.vr === null && !sourceState.installable && !sourceState.source; $("searchCount").textContent = browsing ? "" : `${apps.length} ${apps.length===1 ? "app" : "apps"}${$("sourceQ").value.trim() ? ` for “${$("sourceQ").value.trim()}”` : ""}`; if (!apps.length) { $("searchGrid").innerHTML=storeEmpty(); return; } if (!browsing) { $("searchGrid").innerHTML=`
${apps.map(storeCard).join("")}
`; return; } - const featured=apps.find(a=>a.offers[0].vr && a.offers[0].artwork?.banner && a.offers[0].verdict?.tone !== 'blocked') || apps.find(a=>a.offers[0].artwork?.banner) || apps[0], o=featured.offers[0]; - const tested=apps.filter(a=>a.offers.some(o=>o.vr && o.verdict?.tone==='works')); - const vr=tested.length ? tested : apps.filter(a=>a.offers.some(o=>o.vr && o.verdict?.tone !== 'blocked')); - const popular=[...apps].sort((a,b)=>(b.offers[0].popularity||0)-(a.offers[0].popularity||0)); - const recent=apps.filter(a=>a.offers.some(o=>o.updated)).sort((a,b)=>String(b.offers[0].updated||'').localeCompare(String(a.offers[0].updated||''))); - $("searchGrid").innerHTML=`` + storeRow(apps.some(a=>a.offers[0].popularity) ? "Popular apps" : "Explore apps","More ways to make your Frame yours",popular) + storeRow(tested.length ? "VR that works on the Frame" : "Step into VR",tested.length ? "Experiences checked on a Frame" : "A new perspective, a whole new playground",vr) + storeRow("Recently updated","Fresh from their creators",recent); + // The featured app: something that runs, with real artwork, and preferably VR. It isn't repeated below. + const runs=a=>a.offers[0].verdict?.tone!=='blocked', art=a=>a.offers[0].artwork?.banner; + const featured=apps.find(a=>a.offers[0].vr && art(a) && runs(a)) || apps.find(a=>art(a) && runs(a)) || apps.find(runs) || apps[0], o=featured.offers[0]; + const rest=apps.filter(a=>a!==featured); + const tested=rest.filter(a=>a.offers.some(o=>o.vr && o.verdict?.tone==='works')); + const vr=tested.length ? tested : rest.filter(a=>a.offers.some(o=>o.vr && o.verdict?.tone !== 'blocked')); + // Rows don't repeat each other either: each takes the next apps not shown yet. + const shown=new Set(), take=list=>{ const out=list.filter(a=>!shown.has(a)).slice(0,5); out.forEach(a=>shown.add(a)); return out; }; + const vrRow=take(vr); + const likely={works:0,ready:1,unknown:2,blocked:3}, rank=a=>likely[a.offers[0].verdict?.tone] ?? 2; + const popular=take([...rest].sort((a,b)=>rank(a)-rank(b) || (b.offers[0].popularity||0)-(a.offers[0].popularity||0))); + const recent=take(rest.filter(a=>a.offers.some(o=>o.updated)).sort((a,b)=>String(b.offers[0].updated||'').localeCompare(String(a.offers[0].updated||'')))); + const canInstall=o.downloadable && runs(featured); + $("searchGrid").innerHTML=`
${storeArt(o,"",true)}
Featured${o.vr ? " in VR" : ""}${esc(featured.name)}${esc(storeText(featured.summary))}${storeRuns(o)}${canInstall ? `` : ""}
` + + storeRow(tested.length ? "VR that works on the Frame" : "VR games and experiences", tested.length ? "Checked on a Frame" : "Made for headsets; most aren't checked on the Frame yet", vrRow) + + storeRow(apps.some(a=>a.offers[0].popularity) ? "Popular" : "More to try", "", popular) + + storeRow("Recently updated", "New versions from their developers", recent); } function storeChips() { $("searchFilters").querySelectorAll("button").forEach(b=>b.setAttribute("aria-pressed",b.dataset.filter==='all' ? sourceState.vr===null && !sourceState.installable : b.dataset.filter==='installable' ? sourceState.installable : sourceState.vr===(b.dataset.filter==='vr'))); } async function searchSources(quiet) { @@ -3217,20 +3266,20 @@ async function searchSources(quiet) { result.sources.filter(s=>s.status==='limited').forEach(s=>notes.push(`${s.error}.`)); const stale=result.sources.filter(s=>s.stale).map(s=>storeName(s.name)); if(stale.length)notes.push(`Showing saved listings from ${stale.join(" and ")} while ${stale.length===1 ? "it refreshes" : "they refresh"}.`); - $("sourceStatus").innerHTML=notes.map(esc).join(" ")+(result.elsewhere||[]).map(x=>` Browse ${esc(storeName(x.name))} ↗`).join(""); - $("sourceStatus").hidden=!notes.length && !(result.elsewhere||[]).length; + $("sourceStatus").textContent=notes.join(" "); + $("sourceStatus").hidden=!notes.length; clearTimeout(sourceState.retry); if(loading.length)sourceState.retry=setTimeout(()=>{ if(request===sourceState.request)searchSources(true); },6000); renderStore();await loadSources(); - } catch(e) { if(request===sourceState.request) { $("searchGrid").innerHTML=storeEmpty();$("sourceStatus").hidden=false;$("sourceStatus").textContent="We couldn't reach the app shelves. Try again in a moment."; } } + } catch(e) { if(request===sourceState.request) { $("searchGrid").innerHTML=storeEmpty("Couldn't reach the app sources. Try again in a moment.");$("sourceStatus").hidden=true; } } finally { if(request===sourceState.request)$("searchGrid").setAttribute("aria-busy","false"); } } function currentOffer() { return sourceState.detail?.app.offers[sourceState.detail.choice]; } -function detailBadges(o) { return `${o.vr ? "VR experience" : o.vr===false ? "Flat-screen app" : "Android app"}${o.verdict?.tone==='works' ? '✓ Works on Frame' : ''}Free${storeOpenSource(o) ? 'Open source' : ''}`; } -function renderDetail() { +function detailBadges(o) { return `${storeKind(o)}${storeRuns(o)}Free${storeOpenSource(o) ? 'Open source' : ''}`; } +function renderAppDetail() { const restoreOfferFocus=document.activeElement?.name==='storeOffer'; const d=sourceState.detail;if(!d)return;const o=currentOffer(), images=o.artwork?.screenshots||[], page=sourcePage(o.page); - $("detailBody").innerHTML=storeArt(o,"detail-banner",true)+`
${storeIcon(o)}

${esc(o.name)}

${esc(o.developer || o.author || 'From '+storeName(o.source_name))}

${detailBadges(o)}
${images.length ? `
${images.length} screenshots · scroll to explore
` : ''}

About this app

${esc(storeText(o.description || o.summary || "Discover this app on its developer's page."))}
${page ? `

Meet the creators ↗

` : ''}
Technical details
Package
${esc(o.package||"Unknown")}
Version
${esc(o.version||"Unknown")} (${esc(o.version_code??"unknown")})
Android API
${esc(o.min_sdk??"Unknown")}
Architecture
${esc(o.abis?.join(', ')||"Unknown / no native code")}
Updated
${esc(o.updated||"Unknown")}
Compatibility
${(o.fit?.reasons||[]).map(esc).join(' · ')||'No known installation blockers'}
`; + $("detailBody").innerHTML=storeArt(o,"detail-banner",true)+`
${storeIcon(o)}

${esc(o.name)}

${esc(o.developer || o.author || 'From '+storeName(o.source_name))}

${detailBadges(o)}
${images.length ? `
${images.length} screenshots · scroll to explore
` : ''}

About this app

${esc(storeText(o.description || o.summary || "Discover this app on its developer's page."))}
${page ? `

The developer's page ↗

` : ''}
Technical details
Package
${esc(o.package||"Unknown")}
Version
${esc(o.version||"Unknown")} (${esc(o.version_code??"unknown")})
Android API
${esc(o.min_sdk??"Unknown")}
Architecture
${esc(o.abis?.join(', ')||"Unknown / no native code")}
Updated
${esc(o.updated||"Unknown")}
Compatibility
${(o.fit?.reasons||[]).map(esc).join(' · ')||'No known installation blockers'}
`; updateInstall(); if(restoreOfferFocus)document.querySelector('#detailOffers input:checked')?.focus(); } @@ -3238,16 +3287,17 @@ function updateInstall() { const o=currentOffer();if(!o || !$("detailInstall"))return; const job=sourceState.jobs.get(`${o.source}:${o.id}`), blocked=o.verdict?.tone==='blocked', page=sourcePage(o.page), b=$("detailInstall"); b.disabled=!!(job && !job.done) || !!job?.addingData || (blocked && !job?.result) || (!o.downloadable && !page); - b.textContent=job?.result?.game_data ? (job.addingData ? 'Adding game data…' : 'Add game data') : job?.result ? 'Open in Steam' : job && !job.done ? (job.stage||'Downloading')+(job.percent!=null?` ${Math.round(job.percent)}%`:'…') : blocked ? 'Not compatible' : o.downloadable ? (job?.error?'Try again':'Install on Frame') : 'Get from the creator ↗'; + b.textContent=job?.result?.game_data ? (job.addingData ? 'Adding game data…' : 'Add game data') : job?.result ? 'Open in Steam' : job && !job.done ? (job.stage||'Downloading')+(job.percent!=null?` ${Math.round(job.percent)}%`:'…') : blocked ? 'Not compatible' : o.downloadable ? (job?.error?'Try again':'Install on the Frame') : 'Get from the creator ↗'; $("detailProgress").hidden=!job || job.done; if(job?.percent!=null)$("detailProgress").value=job.percent;else $("detailProgress").removeAttribute("value"); - $("detailInstallNote").textContent=job?.result?.game_data ? (job.dataError || 'Installed. It also needs its game data: open it once on the Frame, then choose Add game data.') : job?.result ? '✓ Installed. Find it in your Steam library.' : job?.error || (job && !job.done ? 'Keep exploring. We’ll take care of the installation.' : o.verdict?.tone==='works' ? 'Adds to your Steam library.' : 'Compatibility can vary. Adds to your Steam library.'); + $("detailInstallNote").textContent=job?.result?.game_data ? (job.dataError || 'Installed. It also needs its game data: open it once on the Frame, then choose Add game data.') : job?.result ? '✓ Installed. Find it in your Steam library.' : job?.error || (job && !job.done ? 'Installing. You can close this and keep browsing.' : o.verdict?.tone==='works' ? 'Adds to your Steam library.' : 'Compatibility can vary. Adds to your Steam library.'); $("detailOffers").querySelectorAll('input').forEach(input=>input.disabled=!!job&&!job.done); } -async function openStoreApp(index) { +async function openStoreApp(index, install=false) { const app=sourceState.apps[index];if(!app)return; sourceState.detail={app:{...app,offers:app.offers.map(o=>({...o}))},choice:0}; - renderDetail();if(!$("appDetail").open)$("appDetail").showModal();$("appDetail").scrollTop=0; + renderAppDetail();if(!$("appDetail").open)$("appDetail").showModal();$("appDetail").scrollTop=0; + if(install)installStoreApp(); // the hero's Install: the details show its progress await loadOfferDetails(); } async function loadOfferDetails() { @@ -3256,7 +3306,7 @@ async function loadOfferDetails() { const detail=await api('/api/sources/details?'+new URLSearchParams({source:o.source,id:o.id})); if(request!==sourceState.detailRequest || d!==sourceState.detail)return; const artwork={icon:detail.artwork?.icon||o.artwork?.icon,banner:detail.artwork?.banner||o.artwork?.banner,screenshots:detail.artwork?.screenshots?.length?detail.artwork.screenshots:o.artwork?.screenshots||[]}; - d.app.offers[d.choice]={...o,...detail,artwork};renderDetail(); + d.app.offers[d.choice]={...o,...detail,artwork};renderAppDetail(); } catch(e) { if(d===sourceState.detail)$("detailInstallNote").textContent="Some app details couldn't be refreshed. You can still view this listing."; } } async function installStoreApp() { @@ -3284,11 +3334,10 @@ async function changeSource(body) { let storeSearchTimer; $("sourceQ").oninput=()=>{clearTimeout(storeSearchTimer);storeSearchTimer=setTimeout(searchSources,300);}; $("sourceSearch").onsubmit=e=>{e.preventDefault();clearTimeout(storeSearchTimer);searchSources();}; -$("sourceFilter").onchange=e=>{sourceState.source=e.target.value;searchSources();}; $("searchFilters").onclick=e=>{const b=e.target.closest('[data-filter]');if(!b)return;const f=b.dataset.filter;if(f==='all'){sourceState.vr=null;sourceState.installable=false;}else if(f==='installable')sourceState.installable=!sourceState.installable;else sourceState.vr=sourceState.vr===(f==='vr')?null:f==='vr';storeChips();searchSources();}; -$("searchGrid").onclick=e=>{const b=e.target.closest('[data-app]');if(b)openStoreApp(+b.dataset.app);if(e.target.closest('#storeReset')){$('sourceQ').value='';sourceState.vr=null;sourceState.installable=false;sourceState.source='';storeChips();searchSources();}}; +$("searchGrid").onclick=e=>{const h=e.target.closest('[data-hero-install]');if(h){openStoreApp(+h.dataset.heroInstall,true);return;}const b=e.target.closest('[data-app]');if(b)openStoreApp(+b.dataset.app);if(e.target.closest('#storeReset')){$('sourceQ').value='';sourceState.vr=null;sourceState.installable=false;sourceState.source='';storeChips();searchSources();}}; $("detailBody").onclick=e=>{if(e.target.closest('#detailInstall'))installStoreApp();const b=e.target.closest('[data-gallery]');if(b)$('detailGallery').scrollBy({left:+b.dataset.gallery*$('detailGallery').clientWidth*.86,behavior:matchMedia('(prefers-reduced-motion: reduce)').matches?'instant':'smooth'});}; -$("detailBody").onchange=e=>{if(e.target.name==='storeOffer'){sourceState.detail.choice=+e.target.value;renderDetail();loadOfferDetails();}}; +$("detailBody").onchange=e=>{if(e.target.name==='storeOffer'){sourceState.detail.choice=+e.target.value;renderAppDetail();loadOfferDetails();}}; $("sourcesOpen").onclick=()=>{loadSources();$('sourcesDialog').showModal();}; $("sourcesList").onchange=e=>{if(e.target.matches('[data-enable]'))changeSource({action:'enable',source:e.target.dataset.enable,enabled:e.target.checked});}; $("sourcesList").onclick=e=>{const b=e.target.closest('[data-remove]');if(b)changeSource({action:'remove',source:b.dataset.remove});}; @@ -3707,7 +3756,7 @@ $("shotsFolder").onclick = e => act($("shotsFolder").textContent, () => api("/ap // ---- Panel switcher: our UI over SteamVR's panel API ---- let panelSeq = 0; -async function loadPanels() { +async function loadPanelSwitcher() { const seq = ++panelSeq; try { const s = await api("/api/panels", {action: "list"}); @@ -3718,7 +3767,7 @@ async function loadPanels() { : '
No open panels.
'; } catch (e) { if (seq === panelSeq) failed($("panelList"), e); } } -$("panelsRefresh").onclick = loadPanels; +$("panelsRefresh").onclick = loadPanelSwitcher; $("panelsOpen").onclick = e => act("Open panel switcher in headset", () => api("/api/panels", {action: "open"}), e.currentTarget); $("panelList").onclick = e => { @@ -3819,7 +3868,7 @@ function showPage() { document.title = page === "home" ? "Frame Control" : `${page[0].toUpperCase() + page.slice(1)} · Frame Control`; const section = !PAGES.includes(id) && id && $(id); if (section) section.scrollIntoView(); else scrollToY(0); - if (page === "tools") { loadMacView(); loadPanels(); } + if (page === "tools") { loadMacView(); loadPanelSwitcher(); } } window.addEventListener("hashchange", showPage); // While a text field has focus, phones hide the bottom tab bar (see body.typing in the CSS). From 497b5d9ae67e9e31bef7f347cee285275716c56c Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:21:34 +1000 Subject: [PATCH 2/5] Don't launch from the hero's Install, and check page names with V8 From the independent review of this PR: - After installing the featured app, the hero's Install on the Frame took the installed branch and launched it on the headset. It now opens the details, whose button says Open in Steam, without acting. - The duplicate-name test matched declarations at column zero only, so an indented second loadPanels (the bug it was written for) passed, and so did classes, destructuring and later declarators. It now compiles both inline scripts as one strict-mode block, where V8 rejects any name declared twice in the shared scope, with fixtures for each case and for same-named helpers in different functions (which are fine). - A card's "Won't run" overstated the check's "Might not work"; it now says May not run. - Removed styles and the source-filter state that nothing used any more. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_page_scripts.py | 41 ++++++++++++++++++++++++++++++++------ ui/index.html | 20 +++++++++---------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/tests/test_page_scripts.py b/tests/test_page_scripts.py index 35ba048..f04468c 100644 --- a/tests/test_page_scripts.py +++ b/tests/test_page_scripts.py @@ -1,21 +1,50 @@ """The page's inline scripts share one global scope, so a second top-level function or variable with a name already used replaces the first everywhere, silently.""" -import collections +import json import pathlib import re +import shutil +import subprocess import unittest ROOT = pathlib.Path(__file__).resolve().parents[1] +# Compiles the scripts as one strict-mode block. There, functions are block scoped like let +# and const, so V8 itself rejects a name declared twice in the shared scope (however it's +# indented or declared: function, class, let, const, destructuring), while helpers with the +# same name inside different functions stay legal. +CHECK = r''' +const vm = require('vm'); +const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8')); +try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); } +catch (e) { console.log(e.message); } +''' + +def check(scripts): + r = subprocess.run(['node', '-e', CHECK], input=json.dumps(scripts), capture_output=True, text=True) + return r.stdout.strip() or r.stderr.strip() + + +@unittest.skipUnless(shutil.which('node'), 'Node parses the page scripts') class PageScripts(unittest.TestCase): def test_no_top_level_name_is_declared_twice(self): page = (ROOT / 'ui/index.html').read_text(encoding='utf-8') - names = collections.Counter() - for js in re.findall(r'', page, re.S): - for m in re.finditer(r'^(?:async\s+)?function\s+(\w+)|^(?:const|let|var)\s+(\w+)\s*=', js, re.M): - names[m.group(1) or m.group(2)] += 1 - self.assertEqual({k: n for k, n in names.items() if n > 1}, {}) + self.assertEqual(check(re.findall(r'', page, re.S)), 'ok') + + def test_the_check_finds_what_it_should(self): + twice = { + 'indented function': ['function loadPanels() {}', ' async function loadPanels() {}'], + 'class': ['class Panel {}', 'class Panel {}'], + 'destructured': ['const { a, b } = {};', 'let [b] = [];'], + 'later declarator': ['let x = 1;', 'const y = 2, x = 3;'], + 'function and const': ['function f() {}', 'const f = 1;'], + } + for what, scripts in twice.items(): + with self.subTest(what): + self.assertIn('already been declared', check(scripts)) + helpers = ['function a() { function help() {} }', 'function b() { const help = 1; }'] + self.assertEqual(check(helpers), 'ok') if __name__ == '__main__': diff --git a/ui/index.html b/ui/index.html index 69e61ed..5404c6c 100644 --- a/ui/index.html +++ b/ui/index.html @@ -575,7 +575,6 @@ .store-chips { display:flex; flex-wrap:wrap; gap:7px; } .store-chips button { border-radius:20px; font-size:12px; height:34px; padding:0 14px; background:#ffffff07; color:#b5c0cb; border:1px solid #ffffff0c; } .store-chips button[aria-pressed=true] { background:#235779; border-color:#5287a5; color:white; } - .store-source-select select { max-width:180px; padding:8px 26px 8px 10px; font:inherit; font-size:12px; border:0; border-radius:5px; color:var(--text); background:#1d2b3b; } .store-notice { color:#a8b5c2; font-size:12px; margin:12px 0; } .store-heading a, .cross-link a { color:var(--link); } .cross-link { margin:-6px 0 12px; } @@ -619,10 +618,8 @@ .store-card-title strong { font-size:15px; color:var(--bright); text-overflow:ellipsis; overflow:hidden; white-space:nowrap; } .store-summary { display:block; color:#a8b5c2; font-size:12px; white-space:nowrap; overflow:hidden; text-overflow:ellipsis; } .store-card-foot { display:flex; justify-content:space-between; align-items:center; gap:6px; margin-top:16px; font-size:11px; color:#a8b5c2; } - .store-card-foot b { color:#dbeecf; font-weight:500; } .store-badges { display:flex; flex-wrap:wrap; gap:7px; } .store-badge { font-size:11px; color:#b7d8ec; background:#6ec9ff14; padding:4px 9px; border-radius:5px; } - .store-badge.works { color:#b8e2ab; background:#70bb5418; } .store-empty { display:flex; flex-direction:column; align-items:center; text-align:center; padding:58px 20px 70px; } .store-empty svg { width:110px; height:90px; color:var(--link); margin-bottom:14px; } .store-empty h2 { font-size:23px; color:var(--bright); margin:8px 0; text-transform:none; letter-spacing:0; } @@ -3189,7 +3186,7 @@ document.body.addEventListener("click", async e => { loadOwned(); // ---- Discover apps ---- -const sourceState = { vr: null, installable: false, source: "", apps: [], sources: [], request: 0, detail: null, detailRequest: 0, jobs: new Map() }; +const sourceState = { vr: null, installable: false, apps: [], sources: [], request: 0, detail: null, detailRequest: 0, jobs: new Map() }; const storeName = s => String(s || "").replace(/ \(demo\)$/i, ""); const storeHue = name => [...String(name)].reduce((n, c) => (n * 31 + c.charCodeAt(0)) >>> 0, 7) % 360; const storeInitial = name => [...String(name || "?")][0].toUpperCase(); // the small icon's stand-in @@ -3230,7 +3227,7 @@ async function loadSources() { } catch (e) { $("sourceError").textContent = e.message; } } // Will it run? Short enough for a card; the full sentence is in its tooltip and the details. -const RUNS = { works: ["✓ Works on the Frame", "works"], ready: ["Should install", "ready"], unknown: ["Not checked yet", "unknown"], blocked: ["Won't run", "blocked"] }; +const RUNS = { works: ["✓ Works on the Frame", "works"], ready: ["Should install", "ready"], unknown: ["Not checked yet", "unknown"], blocked: ["May not run", "blocked"] }; function storeRuns(o) { const [text, cls] = RUNS[o.verdict?.tone] || RUNS.unknown; return `${text}`; @@ -3245,7 +3242,7 @@ function storeEmpty(message) { return `

${message ? "Couldn't load apps" : "No apps found"}

${message ? esc(message) : "Try another name, or clear the filters."}

`; } function renderStore() { - const apps=sourceState.apps, browsing=!$("sourceQ").value.trim() && sourceState.vr === null && !sourceState.installable && !sourceState.source; + const apps=sourceState.apps, browsing=!$("sourceQ").value.trim() && sourceState.vr === null && !sourceState.installable; $("searchCount").textContent = browsing ? "" : `${apps.length} ${apps.length===1 ? "app" : "apps"}${$("sourceQ").value.trim() ? ` for “${$("sourceQ").value.trim()}”` : ""}`; if (!apps.length) { $("searchGrid").innerHTML=storeEmpty(); return; } if (!browsing) { $("searchGrid").innerHTML=`
${apps.map(storeCard).join("")}
`; return; } @@ -3272,7 +3269,7 @@ async function searchSources(quiet) { const request=++sourceState.request; $("searchGrid").setAttribute("aria-busy","true"); if(!quiet)$("searchGrid").innerHTML=`
${Array.from({length:8},()=>'').join("")}
`; - const params=new URLSearchParams({q:$("sourceQ").value,source:sourceState.source,installable:String(sourceState.installable)}); + const params=new URLSearchParams({q:$("sourceQ").value,installable:String(sourceState.installable)}); if(sourceState.vr!==null)params.set("vr",String(sourceState.vr)); try { const result=await api("/api/search?"+params);if(request!==sourceState.request)return; @@ -3316,7 +3313,10 @@ async function openStoreApp(index, install=false) { const app=sourceState.apps[index];if(!app)return; sourceState.detail={app:{...app,offers:app.offers.map(o=>({...o}))},choice:0}; renderAppDetail();if(!$("appDetail").open)$("appDetail").showModal();$("appDetail").scrollTop=0; - if(install)installStoreApp(); // the hero's Install: the details show its progress + // The hero's Install: the details show its progress. Already installed or installing: just + // the details, whose button says what it does (Open in Steam would launch it). + const o=currentOffer(); + if(install&&!sourceState.jobs.has(`${o.source}:${o.id}`))installStoreApp(); await loadOfferDetails(); } async function loadOfferDetails() { @@ -3347,7 +3347,7 @@ async function installStoreApp() { } async function changeSource(body) { $("sourceError").textContent="Saving your sources…"; - try { const result=await api('/api/sources',body);$("sourceError").textContent=result.message;sourceState.source='';await loadSources();await searchSources(); } + try { const result=await api('/api/sources',body);$("sourceError").textContent=result.message;await loadSources();await searchSources(); } catch(e) { $("sourceError").textContent=e.message;await loadSources(); } } let storeSearchTimer; @@ -3365,7 +3365,7 @@ async function addSource(body) { let error=""; const result=await runJob('Add repository',`repo:${body.url}`,()=>api('/api/sources',body).catch(e=>{error=e.message;throw e;}),j=>{error=j.error||"";}); $("sourceError").textContent=result ? result.message : error || "We couldn't add that repository."; - if(result){$("addSource").reset();sourceState.source='';await loadSources();await searchSources();} + if(result){$("addSource").reset();await loadSources();await searchSources();} } $("addSource").onsubmit=e=>{e.preventDefault();const f=new FormData(e.target);addSource({action:'add',url:f.get('url'),fingerprint:f.get('fingerprint')});}; document.querySelectorAll('[data-close]').forEach(b=>b.onclick=()=>$(b.dataset.close).close()); From 0d181b676e88ec77f8bc45a132cca2886e518dc3 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:04:52 +1000 Subject: [PATCH 3/5] Retry from the hero after a failed install, and keep var out of the page From the fix review: - The hero's Install skipped installing whenever a job existed, including one that had failed, so it only opened the details. It now tries again after a failure, and still only opens the details while installing or once installed. - Declaring a var twice is legal, so the strict-mode check couldn't see it. The page's one var (a block-local in the capture code) is now let, and the test rejects var outright (CSS var() in strings is fine). It also checks every inline script, whatever its attributes. - Clear filters still set the removed source filter. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_page_scripts.py | 12 ++++++++++-- ui/index.html | 11 ++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/test_page_scripts.py b/tests/test_page_scripts.py index f04468c..afc44bf 100644 --- a/tests/test_page_scripts.py +++ b/tests/test_page_scripts.py @@ -12,10 +12,13 @@ ROOT = pathlib.Path(__file__).resolve().parents[1] # Compiles the scripts as one strict-mode block. There, functions are block scoped like let # and const, so V8 itself rejects a name declared twice in the shared scope (however it's # indented or declared: function, class, let, const, destructuring), while helpers with the -# same name inside different functions stay legal. +# same name inside different functions stay legal. var is the exception (declaring one twice +# is allowed), so the page doesn't use var at all. CHECK = r''' const vm = require('vm'); const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8')); +const uses = scripts.join('\n').match(/(^|[^\w$.])var\s+[\w${[]/); +if (uses) { console.log('declares with var: ' + uses[0].trim()); process.exit(0); } try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); } catch (e) { console.log(e.message); } ''' @@ -30,7 +33,10 @@ def check(scripts): class PageScripts(unittest.TestCase): def test_no_top_level_name_is_declared_twice(self): page = (ROOT / 'ui/index.html').read_text(encoding='utf-8') - self.assertEqual(check(re.findall(r'', page, re.S)), 'ok') + # Every inline script, whatever its attributes (not src= ones, which aren't inline). + scripts = re.findall(r']*\bsrc=)[^>]*>(.*?)', page, re.S) + self.assertGreaterEqual(len(scripts), 2) + self.assertEqual(check(scripts), 'ok') def test_the_check_finds_what_it_should(self): twice = { @@ -40,6 +46,8 @@ class PageScripts(unittest.TestCase): 'later declarator': ['let x = 1;', 'const y = 2, x = 3;'], 'function and const': ['function f() {}', 'const f = 1;'], } + self.assertIn('var', check(['var x = 1;', 'var x = 2;'])) # legal JavaScript, so: no var + self.assertEqual(check(['const css = "color: var(--blue)";']), 'ok') # CSS var() isn't one for what, scripts in twice.items(): with self.subTest(what): self.assertIn('already been declared', check(scripts)) diff --git a/ui/index.html b/ui/index.html index 405b04b..2fb244b 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1997,7 +1997,7 @@ async function capture() { let url = null; try { let url_ = "/api/screenshot?view=headset"; - var shotPanel = null; + let shotPanel = null; if (view === "flat") { shotPanel = deskPanel() || (await loadPanels(), deskPanel()); if (!shotPanel) throw new Error("no app panel is open in the headset"); @@ -3314,9 +3314,10 @@ async function openStoreApp(index, install=false) { sourceState.detail={app:{...app,offers:app.offers.map(o=>({...o}))},choice:0}; renderAppDetail();if(!$("appDetail").open)$("appDetail").showModal();$("appDetail").scrollTop=0; // The hero's Install: the details show its progress. Already installed or installing: just - // the details, whose button says what it does (Open in Steam would launch it). - const o=currentOffer(); - if(install&&!sourceState.jobs.has(`${o.source}:${o.id}`))installStoreApp(); + // the details, whose button says what it does (Open in Steam would launch it). After a + // failed install it tries again. + const o=currentOffer(), job=sourceState.jobs.get(`${o.source}:${o.id}`); + if(install&&(!job||job.done&&!job.result))installStoreApp(); await loadOfferDetails(); } async function loadOfferDetails() { @@ -3354,7 +3355,7 @@ let storeSearchTimer; $("sourceQ").oninput=()=>{clearTimeout(storeSearchTimer);storeSearchTimer=setTimeout(searchSources,300);}; $("sourceSearch").onsubmit=e=>{e.preventDefault();clearTimeout(storeSearchTimer);searchSources();}; $("searchFilters").onclick=e=>{const b=e.target.closest('[data-filter]');if(!b)return;const f=b.dataset.filter;if(f==='all'){sourceState.vr=null;sourceState.installable=false;}else if(f==='installable')sourceState.installable=!sourceState.installable;else sourceState.vr=sourceState.vr===(f==='vr')?null:f==='vr';storeChips();searchSources();}; -$("searchGrid").onclick=e=>{const h=e.target.closest('[data-hero-install]');if(h){openStoreApp(+h.dataset.heroInstall,true);return;}const b=e.target.closest('[data-app]');if(b)openStoreApp(+b.dataset.app);if(e.target.closest('#storeReset')){$('sourceQ').value='';sourceState.vr=null;sourceState.installable=false;sourceState.source='';storeChips();searchSources();}}; +$("searchGrid").onclick=e=>{const h=e.target.closest('[data-hero-install]');if(h){openStoreApp(+h.dataset.heroInstall,true);return;}const b=e.target.closest('[data-app]');if(b)openStoreApp(+b.dataset.app);if(e.target.closest('#storeReset')){$('sourceQ').value='';sourceState.vr=null;sourceState.installable=false;storeChips();searchSources();}}; $("detailBody").onclick=e=>{if(e.target.closest('#detailInstall'))installStoreApp();const b=e.target.closest('[data-gallery]');if(b)$('detailGallery').scrollBy({left:+b.dataset.gallery*$('detailGallery').clientWidth*.86,behavior:matchMedia('(prefers-reduced-motion: reduce)').matches?'instant':'smooth'});}; $("detailBody").onchange=e=>{if(e.target.name==='storeOffer'){sourceState.detail.choice=+e.target.value;renderAppDetail();loadOfferDetails();}}; $("sourcesOpen").onclick=()=>{loadSources();$('sourcesDialog').showModal();}; From 9bd6890fe83b230791142e99232dffe73e14d400 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:29:12 +1000 Subject: [PATCH 4/5] Find var declarations with a parser, and check only classic inline scripts The fix review showed the var regex flagged the word in comments and strings and missed var/*c*/x and var {x}. The test now uses the acorn parser Node ships (--expose-internals; it skips if a Node ever drops it) to find real var declarations, and reads the page with html.parser so it checks inline classic scripts only: no src, and no module or JSON data type. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_page_scripts.py | 80 ++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 11 deletions(-) diff --git a/tests/test_page_scripts.py b/tests/test_page_scripts.py index afc44bf..da48b13 100644 --- a/tests/test_page_scripts.py +++ b/tests/test_page_scripts.py @@ -1,8 +1,8 @@ """The page's inline scripts share one global scope, so a second top-level function or variable with a name already used replaces the first everywhere, silently.""" +import html.parser import json import pathlib -import re import shutil import subprocess import unittest @@ -13,28 +13,75 @@ ROOT = pathlib.Path(__file__).resolve().parents[1] # and const, so V8 itself rejects a name declared twice in the shared scope (however it's # indented or declared: function, class, let, const, destructuring), while helpers with the # same name inside different functions stay legal. var is the exception (declaring one twice -# is allowed), so the page doesn't use var at all. +# is allowed), so the page doesn't use var at all: Node's own copy of the acorn parser finds +# real var declarations, not the word in comments, strings or CSS var(). CHECK = r''' const vm = require('vm'); const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8')); -const uses = scripts.join('\n').match(/(^|[^\w$.])var\s+[\w${[]/); -if (uses) { console.log('declares with var: ' + uses[0].trim()); process.exit(0); } +let acorn, walk; +try { + acorn = require('internal/deps/acorn/acorn/dist/acorn'); + walk = require('internal/deps/acorn/acorn-walk/dist/walk'); +} catch { console.log('no parser'); process.exit(0); } +for (const code of scripts) { + let found = null; + try { + walk.simple(acorn.parse(code, { ecmaVersion: 'latest' }), { + VariableDeclaration(n) { if (n.kind === 'var' && !found) found = code.slice(n.start, n.end).slice(0, 40); }, + }); + } catch (e) { console.log(e.message); process.exit(0); } + if (found) { console.log('declares with var: ' + found); process.exit(0); } +} try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); } catch (e) { console.log(e.message); } ''' def check(scripts): - r = subprocess.run(['node', '-e', CHECK], input=json.dumps(scripts), capture_output=True, text=True) - return r.stdout.strip() or r.stderr.strip() + r = subprocess.run(['node', '--expose-internals', '-e', CHECK], input=json.dumps(scripts), + capture_output=True, text=True) + out = r.stdout.strip() or r.stderr.strip() + if out == 'no parser': + raise unittest.SkipTest("this Node doesn't include acorn") + return out + + +class ClassicScripts(html.parser.HTMLParser): + """The page's inline classic scripts: no src, and no type other than JavaScript (a module + has its own scope, and JSON data isn't code).""" + JS = {'', 'text/javascript', 'application/javascript'} + + def __init__(self): + super().__init__() + self.scripts, self.current = [], None + + def handle_starttag(self, tag, attrs): + if tag == 'script': + a = dict(attrs) + inline = 'src' not in a and (a.get('type') or '').strip().lower() in self.JS + self.current = [] if inline else None + + def handle_data(self, data): + if self.current is not None: + self.current.append(data) + + def handle_endtag(self, tag): + if tag == 'script' and self.current is not None: + self.scripts.append(''.join(self.current)) + if tag == 'script': + self.current = None + + +def classic_scripts(page): + p = ClassicScripts() + p.feed(page) + return p.scripts @unittest.skipUnless(shutil.which('node'), 'Node parses the page scripts') class PageScripts(unittest.TestCase): def test_no_top_level_name_is_declared_twice(self): - page = (ROOT / 'ui/index.html').read_text(encoding='utf-8') - # Every inline script, whatever its attributes (not src= ones, which aren't inline). - scripts = re.findall(r']*\bsrc=)[^>]*>(.*?)', page, re.S) + scripts = classic_scripts((ROOT / 'ui/index.html').read_text(encoding='utf-8')) self.assertGreaterEqual(len(scripts), 2) self.assertEqual(check(scripts), 'ok') @@ -46,13 +93,24 @@ class PageScripts(unittest.TestCase): 'later declarator': ['let x = 1;', 'const y = 2, x = 3;'], 'function and const': ['function f() {}', 'const f = 1;'], } - self.assertIn('var', check(['var x = 1;', 'var x = 2;'])) # legal JavaScript, so: no var - self.assertEqual(check(['const css = "color: var(--blue)";']), 'ok') # CSS var() isn't one for what, scripts in twice.items(): with self.subTest(what): self.assertIn('already been declared', check(scripts)) helpers = ['function a() { function help() {} }', 'function b() { const help = 1; }'] self.assertEqual(check(helpers), 'ok') + # var: legal to declare twice, so not allowed at all; however it's written. + for code in ['var x = 1;', 'var/*c*/x = 1;', 'var {x} = {x: 1};', 'function f() { var y; }']: + with self.subTest(code): + self.assertIn('declares with var', check([code])) + # The word var elsewhere is fine. + text = ['// var x\nconst a = "var y", b = `var ${a}`, c = "color: var(--blue)", d = {}.var;'] + self.assertEqual(check(text), 'ok') + + def test_only_inline_classic_scripts_are_checked(self): + page = ('' + '' + '') + self.assertEqual(classic_scripts(page), ['let a;', 'let c;']) if __name__ == '__main__': From c2c1cf0168834fdd1db922108e03f340c0444fb4 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:37:43 +1000 Subject: [PATCH 5/5] Check every JavaScript script type, and keep the name check without acorn From the review of the parser change: scripts typed text/ecmascript, application/x-javascript and the other JavaScript MIME types run as classic scripts too, so they're checked now. And the duplicate-name check (V8 only) no longer skips with the var check when a Node lacks acorn. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_page_scripts.py | 50 ++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/tests/test_page_scripts.py b/tests/test_page_scripts.py index da48b13..8ce35bd 100644 --- a/tests/test_page_scripts.py +++ b/tests/test_page_scripts.py @@ -15,9 +15,14 @@ ROOT = pathlib.Path(__file__).resolve().parents[1] # same name inside different functions stay legal. var is the exception (declaring one twice # is allowed), so the page doesn't use var at all: Node's own copy of the acorn parser finds # real var declarations, not the word in comments, strings or CSS var(). -CHECK = r''' +DUPLICATES = r''' const vm = require('vm'); const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8')); +try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); } +catch (e) { console.log(e.message); } +''' +VARS = r''' +const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8')); let acorn, walk; try { acorn = require('internal/deps/acorn/acorn/dist/acorn'); @@ -32,24 +37,36 @@ for (const code of scripts) { } catch (e) { console.log(e.message); process.exit(0); } if (found) { console.log('declares with var: ' + found); process.exit(0); } } -try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); } -catch (e) { console.log(e.message); } +console.log('ok'); ''' +def node(script, scripts, *flags): + r = subprocess.run(['node', *flags, '-e', script], input=json.dumps(scripts), capture_output=True, text=True) + return r.stdout.strip() or r.stderr.strip() + + def check(scripts): - r = subprocess.run(['node', '--expose-internals', '-e', CHECK], input=json.dumps(scripts), - capture_output=True, text=True) - out = r.stdout.strip() or r.stderr.strip() + """'ok', or V8's message for the first name declared twice.""" + return node(DUPLICATES, scripts) + + +def check_vars(scripts): + """'ok', or the first var declaration. Skips where Node doesn't include acorn.""" + out = node(VARS, scripts, '--expose-internals') if out == 'no parser': raise unittest.SkipTest("this Node doesn't include acorn") return out class ClassicScripts(html.parser.HTMLParser): - """The page's inline classic scripts: no src, and no type other than JavaScript (a module - has its own scope, and JSON data isn't code).""" - JS = {'', 'text/javascript', 'application/javascript'} + """The page's inline classic scripts: no src, and no type other than a JavaScript one (a + module has its own scope, and JSON data isn't code). The types a browser runs as script: + https://mimesniff.spec.whatwg.org/#javascript-mime-type""" + JS = {'', 'application/ecmascript', 'application/javascript', 'application/x-ecmascript', + 'application/x-javascript', 'text/ecmascript', 'text/javascript', 'text/javascript1.0', + 'text/javascript1.1', 'text/javascript1.2', 'text/javascript1.3', 'text/javascript1.4', + 'text/javascript1.5', 'text/jscript', 'text/livescript', 'text/x-ecmascript', 'text/x-javascript'} def __init__(self): super().__init__() @@ -85,6 +102,10 @@ class PageScripts(unittest.TestCase): self.assertGreaterEqual(len(scripts), 2) self.assertEqual(check(scripts), 'ok') + def test_the_page_declares_nothing_with_var(self): + scripts = classic_scripts((ROOT / 'ui/index.html').read_text(encoding='utf-8')) + self.assertEqual(check_vars(scripts), 'ok') + def test_the_check_finds_what_it_should(self): twice = { 'indented function': ['function loadPanels() {}', ' async function loadPanels() {}'], @@ -98,19 +119,22 @@ class PageScripts(unittest.TestCase): self.assertIn('already been declared', check(scripts)) helpers = ['function a() { function help() {} }', 'function b() { const help = 1; }'] self.assertEqual(check(helpers), 'ok') + + def test_the_var_check_finds_what_it_should(self): # var: legal to declare twice, so not allowed at all; however it's written. for code in ['var x = 1;', 'var/*c*/x = 1;', 'var {x} = {x: 1};', 'function f() { var y; }']: with self.subTest(code): - self.assertIn('declares with var', check([code])) + self.assertIn('declares with var', check_vars([code])) # The word var elsewhere is fine. text = ['// var x\nconst a = "var y", b = `var ${a}`, c = "color: var(--blue)", d = {}.var;'] - self.assertEqual(check(text), 'ok') + self.assertEqual(check_vars(text), 'ok') def test_only_inline_classic_scripts_are_checked(self): page = ('' '' - '') - self.assertEqual(classic_scripts(page), ['let a;', 'let c;']) + '' + '') + self.assertEqual(classic_scripts(page), ['let a;', 'let c;', 'let d;', 'let e;']) if __name__ == '__main__':