diff --git a/README.md b/README.md index 2ee60cc..a8dd7a9 100644 --- a/README.md +++ b/README.md @@ -2282,6 +2282,8 @@ view would leave a wallet one click from deletion. - Either "Save" stores its URL only if the input still shows that URL when the endpoint answers. Otherwise a flash message says that the URL was not saved + - If the user taps the same "Save" again before the endpoint answers, the + earlier tap stores nothing and shows no flash message - Etherscan: - "Check recipients on Etherscan" checkbox, unticked until the user ticks it diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 8dec61c..9516f64 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -50,6 +50,11 @@ let versionClickTimer = null; // not the one in `state`. let shownNetwork = null; +// Each endpoint "Save" counts its taps here, under the field that it stores. A +// tap stores its URL and shows a flash message only while it is the latest tap +// of its "Save". +const saveTaps = { rpcUrl: 0, blockscoutUrl: 0 }; + // One row per site origin, however many addresses it appears under, each with // an [x] that hands it to onRemove. function renderSiteList(containerId, origins, onRemove) { @@ -347,10 +352,16 @@ function showNewSettings() { // shows `url`, nothing is saved. The flash message then says that the URL was // changed during the check. The save compares the input and sets `url` in // `state` in one step. So a redraw after the comparison shows `url`. -async function saveEndpoint(field, input, url, networkId) { +// +// `tap` is the number of the "Save" tap that checked `url`. If the user taps +// the same "Save" again before this save holds the lock, nothing is stored. If +// the user taps the same "Save" again before the write ends, no flash message +// is shown. +async function saveEndpoint(field, input, url, networkId, tap) { let refused = false; let inputChanged = false; await saveState((stored) => { + if (saveTaps[field] !== tap) return false; refused = state.networkId !== networkId || stored.networkId !== networkId; inputChanged = $(input).value.trim() !== url; @@ -358,6 +369,7 @@ async function saveEndpoint(field, input, url, networkId) { state[field] = url; stored[field] = url; }); + if (saveTaps[field] !== tap) return; if (refused) { showFlash("The URL was not saved because the network changed."); } else if (inputChanged) { @@ -375,6 +387,8 @@ function init(_ctx) { showPhrase.init(); $("btn-save-rpc").addEventListener("click", async () => { + saveTaps.rpcUrl++; + const tap = saveTaps.rpcUrl; const url = $("settings-rpc").value.trim(); if (!url) { showFlash("Please enter an RPC URL."); @@ -395,6 +409,8 @@ function init(_ctx) { }), }); const json = await resp.json(); + // The user has tapped this "Save" again since the check began. + if (tap !== saveTaps.rpcUrl) return; if (json.error) { log.errorf("RPC validation error:", json.error); showFlash("Endpoint returned an error."); @@ -408,6 +424,7 @@ function init(_ctx) { // Cancelled by the popup closing, not failed: see pageClosed in // src/popup/index.js. if (ctx.pageClosed.aborted) return; + if (tap !== saveTaps.rpcUrl) return; // Not the error's message: fetch puts the whole URL, password and // key included, in the message of the error it throws for a URL // with a user name and password or one it cannot parse. @@ -415,10 +432,12 @@ function init(_ctx) { showFlash("Could not reach endpoint."); return; } - await saveEndpoint("rpcUrl", "settings-rpc", url, net.id); + await saveEndpoint("rpcUrl", "settings-rpc", url, net.id, tap); }); $("btn-save-blockscout").addEventListener("click", async () => { + saveTaps.blockscoutUrl++; + const tap = saveTaps.blockscoutUrl; const url = $("settings-blockscout").value.trim(); if (!url) { showFlash("Please enter a Blockscout API URL."); @@ -428,18 +447,26 @@ function init(_ctx) { showFlash("Testing endpoint..."); try { const resp = await debugFetch(url + "/stats"); + if (tap !== saveTaps.blockscoutUrl) return; if (!resp.ok) { showFlash("Endpoint returned HTTP " + resp.status + "."); return; } } catch { if (ctx.pageClosed.aborted) return; + if (tap !== saveTaps.blockscoutUrl) return; // Not the error's message, as for the RPC check above. log.errorf("Blockscout validation failed:", urlOrigin(url)); showFlash("Could not reach endpoint."); return; } - await saveEndpoint("blockscoutUrl", "settings-blockscout", url, net.id); + await saveEndpoint( + "blockscoutUrl", + "settings-blockscout", + url, + net.id, + tap, + ); }); // Each change is made inside the save, holding the lock. So diff --git a/tests/settingsEndpointSave.test.js b/tests/settingsEndpointSave.test.js index d770061..6afd8a4 100644 --- a/tests/settingsEndpointSave.test.js +++ b/tests/settingsEndpointSave.test.js @@ -167,3 +167,138 @@ test.each([ ); }, ); + +// This answer passes both endpoint checks on Ethereum Mainnet. +const PASSING_ANSWER = { + ok: true, + json: async () => ({ result: NETWORKS.mainnet.chainId }), +}; + +// The user types a URL in Settings and taps "Save". Before the endpoint answers, +// the user types a second URL and taps "Save" again. The second check passes +// first. The second URL is stored, and the flash message reads "Saved.". The +// first check then passes or fails. That answer changes nothing +// (https://git.eeqj.de/sneak/AutistMask/issues/698). +test.each([ + { + button: "btn-save-rpc", + input: "settings-rpc", + field: "rpcUrl", + first: "passes", + }, + { + button: "btn-save-rpc", + input: "settings-rpc", + field: "rpcUrl", + first: "fails", + }, + { + button: "btn-save-blockscout", + input: "settings-blockscout", + field: "blockscoutUrl", + first: "passes", + }, + { + button: "btn-save-blockscout", + input: "settings-blockscout", + field: "blockscoutUrl", + first: "fails", + }, +])( + "$button keeps the second tap's result when the first check $first last", + async ({ button, input, field, first }) => { + const page = await bootPopup(unversionedValidProfile()); + await page.click("btn-settings"); + const checks = []; + globalThis.fetch.mockImplementation( + () => + new Promise((resolve, reject) => { + checks.push({ resolve, reject }); + }), + ); + const secondUrl = "https://second.example.invalid"; + + page.node(input).value = "https://first.example.invalid"; + const firstTap = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + page.node(input).value = secondUrl; + const secondTap = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + checks[1].resolve(PASSING_ANSWER); + await Promise.all(secondTap); + await page.settle(); + expect(page.text("flash-msg")).toBe("Saved."); + if (first === "passes") { + checks[0].resolve(PASSING_ANSWER); + } else { + // fetch rejects when it cannot reach the endpoint. + checks[0].reject(new TypeError("Failed to fetch")); + } + await Promise.all(firstTap); + await page.settle(); + + expect(checks).toHaveLength(2); + expect(page.storage.read("autistmask")[field]).toBe(secondUrl); + expect(page.text("flash-msg")).toBe("Saved."); + }, +); + +// The user types a URL in Settings and taps "Save". Another write holds the +// lock. So the save waits for the lock once the check passes. The user then +// taps "Save" again on the same URL. When the lock is free, the first save +// stores nothing and shows no flash message. The second check then passes, and +// the second save stores the URL. +test.each([ + { button: "btn-save-rpc", input: "settings-rpc", field: "rpcUrl" }, + { + button: "btn-save-blockscout", + input: "settings-blockscout", + field: "blockscoutUrl", + }, +])( + "$button stores nothing for an earlier tap still waiting for the lock", + async ({ button, input, field }) => { + const page = await bootPopup(unversionedValidProfile()); + await page.click("btn-settings"); + const storedUrl = page.value(input); + const checks = []; + globalThis.fetch.mockImplementation( + () => + new Promise((resolve) => { + checks.push(resolve); + }), + ); + const typedUrl = "https://typed.example.invalid"; + + page.node(input).value = typedUrl; + const firstTap = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + let releaseLock; + const lockHeld = navigator.locks.request( + "autistmask", + () => + new Promise((resolve) => { + releaseLock = resolve; + }), + ); + checks[0](PASSING_ANSWER); + await page.settle(); + const secondTap = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + releaseLock(); + await lockHeld; + await Promise.all(firstTap); + await page.settle(); + + expect(page.storage.read("autistmask")[field]).toBe(storedUrl); + expect(page.text("flash-msg")).toBe("Testing endpoint..."); + + checks[1](PASSING_ANSWER); + await Promise.all(secondTap); + await page.settle(); + + expect(checks).toHaveLength(2); + expect(page.storage.read("autistmask")[field]).toBe(typedUrl); + expect(page.text("flash-msg")).toBe("Saved."); + }, +);