diff --git a/README.md b/README.md index e20ba2b..a4a7899 100644 --- a/README.md +++ b/README.md @@ -2279,6 +2279,8 @@ view would leave a wallet one click from deletion. no user data and is made whether or not "Use Blockscout" is ticked. - If the network changes before either "Save" stores its URL, the URL is not saved and a flash message says so + - Either "Save" stores its URL only if the input still shows that URL when + the endpoint answers - 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 4b52632..7033234 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -339,18 +339,27 @@ function showNewSettings() { // Another page may have stored a different URL since then. So `url` is set in // the stored record too. "Saved." then means that the stored record holds // `url`. -async function saveEndpoint(field, url, networkId) { +// +// `input` is the id of the field that the user typed `url` in. This page draws +// Settings again after following another page's change to a setting. A redraw +// during the endpoint check puts the URL from `state` back in the field. So if +// the field no longer shows `url`, nothing is saved and no flash message +// appears. The field is compared inside the save, so a redraw after the +// comparison shows `url` from `state`. +async function saveEndpoint(field, input, url, networkId) { let refused = false; + let fieldChanged = false; await saveState((stored) => { refused = state.networkId !== networkId || stored.networkId !== networkId; - if (refused) return false; + fieldChanged = $(input).value.trim() !== url; + if (refused || fieldChanged) return false; state[field] = url; stored[field] = url; }); if (refused) { showFlash("The URL was not saved because the network changed."); - } else { + } else if (!fieldChanged) { showFlash("Saved."); } } @@ -401,7 +410,7 @@ function init(_ctx) { showFlash("Could not reach endpoint."); return; } - await saveEndpoint("rpcUrl", url, net.id); + await saveEndpoint("rpcUrl", "settings-rpc", url, net.id); }); $("btn-save-blockscout").addEventListener("click", async () => { @@ -425,7 +434,7 @@ function init(_ctx) { showFlash("Could not reach endpoint."); return; } - await saveEndpoint("blockscoutUrl", url, net.id); + await saveEndpoint("blockscoutUrl", "settings-blockscout", url, net.id); }); // Each change is made inside the save, holding the lock. So diff --git a/tests/settingsEndpointSave.test.js b/tests/settingsEndpointSave.test.js index 066a06f..fd1b69b 100644 --- a/tests/settingsEndpointSave.test.js +++ b/tests/settingsEndpointSave.test.js @@ -47,3 +47,55 @@ test.each([ expect(page.text("flash-msg")).toBe("Saved."); }, ); + +// The user types a URL in Settings and taps "Save". While the check waits for +// the endpoint's answer, another page turns "Use Blockscout" on. This page +// follows that change and draws Settings again, with the stored URL in the +// field. The check then passes. The typed URL is not stored, and the flash +// message never reads "Saved." +// (https://git.eeqj.de/sneak/AutistMask/issues/695). +test.each([ + { button: "btn-save-rpc", input: "settings-rpc", field: "rpcUrl" }, + { + button: "btn-save-blockscout", + input: "settings-blockscout", + field: "blockscoutUrl", + }, +])( + "$button stores nothing when Settings is drawn again during the check", + async ({ button, input, field }) => { + const page = await bootPopup(unversionedValidProfile()); + await page.click("btn-settings"); + const storedUrl = page.value(input); + let answer; + globalThis.fetch.mockReturnValue( + new Promise((resolve) => { + answer = resolve; + }), + ); + + page.node(input).value = "https://typed.example.invalid"; + const saving = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + // The browser announces this write to this page. + await page.storage.local.set({ + autistmask: { + ...page.storage.read("autistmask"), + useBlockscout: true, + }, + }); + await page.settle(); + expect(page.node("settings-use-blockscout").checked).toBe(true); + // This answer passes both endpoint checks on Ethereum Mainnet. + answer({ + ok: true, + json: async () => ({ result: NETWORKS.mainnet.chainId }), + }); + await Promise.all(saving); + await page.settle(); + + expect(page.storage.read("autistmask")[field]).toBe(storedUrl); + expect(page.value(input)).toBe(storedUrl); + expect(page.text("flash-msg")).not.toBe("Saved."); + }, +);