diff --git a/CHANGELOG.md b/CHANGELOG.md index dad661d9..bb3b7a4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - Copy as Markdown of the [visualizer](https://webperf-snippets.net/visualizer) exported an audit that has a rating and no value, such as Script-Loading, as a metric: a heading with `undefined` and none of its issues or items, and nothing at all when its unit is a score. The export picks its format as the page picks its view, so the audit comes with its issues and its items, and a result the page shows as JSON is exported as JSON instead of "No issues found". +- The panel of the extension stayed empty when the storage brought a theme or a side of the list that it does not know, such as a value synced from another version: it looked for the option of that value in the settings and stopped there. A saved value the panel does not know gives way to its default. --- diff --git a/extension/lib/settings.js b/extension/lib/settings.js index bd05ab34..468645bb 100644 --- a/extension/lib/settings.js +++ b/extension/lib/settings.js @@ -9,6 +9,20 @@ export const DEFAULTS = { theme: 'devtools', side: 'left', sidebar: 'expanded', snippet: null, audits: [], reloadBeforeAudit: true, auditFilter: 'issues' } const SYNCED = new Set(['theme', 'side', 'reloadBeforeAudit', 'auditFilter']) +// What each setting may hold. The storage can bring anything: a value of another version of the +// extension, synced from another Chrome, or one written by hand. A value that fails gives way to +// the default, so the panel never looks for a control that does not exist +const oneOf = (...values) => (value) => values.includes(value) +const VALID = { + theme: oneOf('devtools', 'light', 'dark'), + side: oneOf('left', 'right'), + sidebar: oneOf('expanded', 'collapsed'), + snippet: (value) => value === null || typeof value === 'string', + audits: Array.isArray, + reloadBeforeAudit: (value) => typeof value === 'boolean', + auditFilter: oneOf('errors', 'issues', 'all'), +} + export function createSettings(storage) { const areaOf = (key) => (SYNCED.has(key) ? storage?.sync : storage?.local) const read = async (area, keys) => { @@ -25,7 +39,8 @@ export function createSettings(storage) { read(storage?.sync, keys.filter((k) => SYNCED.has(k))), read(storage?.local, keys.filter((k) => !SYNCED.has(k))), ]) - return { ...DEFAULTS, ...synced, ...local } + const stored = { ...synced, ...local } + return Object.fromEntries(keys.map((key) => [key, key in stored && VALID[key](stored[key]) ? stored[key] : DEFAULTS[key]])) }, async save(key, value) { try { diff --git a/extension/lib/settings.test.js b/extension/lib/settings.test.js index 0f245f43..0af96197 100644 --- a/extension/lib/settings.test.js +++ b/extension/lib/settings.test.js @@ -49,6 +49,18 @@ test('a value saved in the old place of the setting is ignored', async () => { assert.deepEqual(await settings.load(), DEFAULTS) }) +test('a saved value the panel does not know gives way to the default', async () => { + const sync = area({ theme: 'sepia', side: 'top', auditFilter: 'none', reloadBeforeAudit: 'yes' }) + const local = area({ sidebar: 'floating', snippet: 42, audits: 'none' }) + assert.deepEqual(await createSettings({ sync, local }).load(), DEFAULTS) +}) + +test('each value the panel knows is kept', async () => { + const sync = area({ theme: 'light', side: 'right', auditFilter: 'errors', reloadBeforeAudit: false }) + const local = area({ sidebar: 'collapsed', snippet: 'LCP', audits: [] }) + assert.deepEqual(await createSettings({ sync, local }).load(), { ...sync.data, ...local.data }) +}) + test('without chrome.storage, or when it fails, the panel keeps the defaults and saving does nothing', async () => { assert.deepEqual(await createSettings(undefined).load(), DEFAULTS) await createSettings(undefined).save('theme', 'dark') diff --git a/extension/tests/panel.e2e.test.js b/extension/tests/panel.e2e.test.js index 9f1e8680..a245255c 100644 --- a/extension/tests/panel.e2e.test.js +++ b/extension/tests/panel.e2e.test.js @@ -302,6 +302,25 @@ test('the settings choose the theme and the side of the list, and the panel reme assert.equal(await side(), 'left') }) +test('the panel opens with its defaults when the storage brings a theme or a side it does not know', async () => { + await panel.evaluate(() => { + localStorage.setItem('sync:theme', JSON.stringify('sepia')) + localStorage.setItem('sync:side', JSON.stringify('top')) + }) + try { + await panel.reload() + await panel.waitForSelector('#list button', { timeout: 5000 }) + await panel.click('#open-settings') + assert.ok(await panel.isChecked('input[name="theme"][value="devtools"]')) + assert.ok(await panel.isChecked('input[name="side"][value="left"]')) + await panel.click('#settings .close') + } finally { + // The page error of the panel that did not open is what the test reports, not the ones after it + await panel.evaluate(() => ['sync:theme', 'sync:side'].forEach((key) => localStorage.removeItem(key))) + await reopenPanel() + } +}) + test('Clear empties the result, and only shows when there is one', async () => { await target.goto(`${servers.base}/seeded`, { waitUntil: 'load' }) await panel.click('#list button[data-id="Input-Latency-Breakdown"]')