Skip to content

Add ?load= URL param; update URL on ROM selection #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattgodbolt merged 3 commits into from
Mar 3, 2026
Merged

Add ?load= URL param; update URL on ROM selection #61

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3831c64
Add ?load= URL param and update URL on ROM load
mattgodbolt-molty Mar 3, 2026
f89f999
Fix: restore loadUploadFile to use resetLoadAndStart
mattgodbolt-molty Mar 3, 2026
c6d6d78
Address Copilot review: sanitise ?load=, fix hash, fix empty name
mattgodbolt-molty Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,36 @@ function loadRomData(name) {
return request.response;
}

function updateUrl(params) {
const url = new URL(window.location.href);
// Clear existing emulator params before setting new ones
for (const key of [...url.searchParams.keys()]) {
url.searchParams.delete(key);
}
for (const [key, val] of Object.entries(params)) {
if (val !== null) url.searchParams.set(key, val);
}
history.replaceState(null, "", url);
Comment on lines +19 to +29
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateUrl rewrites the query string but leaves url.hash intact. Since parseQuery() merges ?search and #hash with the hash taking precedence (hash is appended last), any existing #b64sms/#load will keep overriding the new query params on reload, making the URL updates ineffective for users coming from hash-based links. Consider clearing url.hash (or updating parseQuery precedence) when you switch to query-param based state.

Copilot uses AI. Check for mistakes.
}

function resetLoadAndStart(filename, romdata) {
miracle_reset();
bus.loadRom(filename, romdata, debug_init);
hideRomChooser();
start();
updateUrl({ load: filename });
}

function loadUploadFile(file) {
const reader = new FileReader();
reader.onload = function () {
resetLoadAndStart(file.name, reader.result);
// Uploaded files use b64sms encoding for shareability
Copy link
Owner

@mattgodbolt mattgodbolt Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is resetLoadAndStart now dead code? looks like you duplicated its work here

Copy link
Collaborator Author

@mattgodbolt-molty mattgodbolt-molty Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm Molty, an AI assistant acting on behalf of @mattgodbolt) Good catch — fixed. resetLoadAndStart now takes an optional urlParams arg; loadUploadFile passes { b64sms, load } through it rather than duplicating the logic.

const b64 = btoa(reader.result);
miracle_reset();
bus.loadRom(file.name, reader.result, debug_init);
hideRomChooser();
start();
updateUrl({ b64sms: b64 });
};
reader.readAsBinaryString(file);
}
Expand Down Expand Up @@ -129,7 +148,13 @@ function go() {

const parsedQuery = parseQuery();
if (parsedQuery["b64sms"]) {
bus.loadRom("b64.sms", atob(parsedQuery["b64sms"]), debug_init);
const name = parsedQuery["load"] ?? "uploaded.sms";
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsedQuery["load"] can be an empty string for URLs like ?load=. In that case this code will pass an empty filename into bus.loadRom, and updateUrl will persist load=; it likely should fall back to the default upload name (or ignore load) when the value is empty.

Suggested change
const name = parsedQuery["load"] ?? "uploaded.sms";
const name = parsedQuery["load"] || "uploaded.sms";

Copilot uses AI. Check for mistakes.
bus.loadRom(name, atob(parsedQuery["b64sms"]), debug_init);
updateUrl({ b64sms: parsedQuery["b64sms"], load: name });
} else if (parsedQuery["load"]) {
const name = parsedQuery["load"];
bus.loadRom(name, loadRomData(name), debug_init);
updateUrl({ load: name });
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name comes directly from the URL and is concatenated into the XHR path in loadRomData ("roms/" + name). With values like ../index.html this will request files outside the roms/ directory. If ?load= is meant to only support known ROMs, validate against RomList (or at least reject path separators / ..) before calling loadRomData.

Copilot uses AI. Check for mistakes.
} else {
const defaultRom = getDefaultRom();
bus.loadRom(defaultRom, loadRomData(defaultRom), debug_init);
Expand Down