From 403ee0470bb4c36eaf4c887e3bbae7da12483836 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Fri, 15 Sep 2023 13:48:55 -0500 Subject: [PATCH 1/5] Fix #20910 - Turn off ENS resolution if IPFS is turned off --- ui/pages/settings/security-tab/security-tab.component.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js index 5452f7b9b4a0..d9b4ef76af04 100644 --- a/ui/pages/settings/security-tab/security-tab.component.js +++ b/ui/pages/settings/security-tab/security-tab.component.js @@ -408,6 +408,7 @@ export default class SecurityTab extends PureComponent { if (value) { // turning from true to false this.props.setIpfsGateway(''); + this.props.setUseAddressBarEnsResolution(false); } else { // turning from false to true handleIpfsGatewayChange(this.state.ipfsGateway); From a549e607d1dd5b5a6965c627bde664d6d7c0969e Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 20 Sep 2023 11:14:43 -0500 Subject: [PATCH 2/5] Updated WIP --- app/scripts/lib/ens-ipfs/setup.js | 18 +++++++++++++++--- .../security-tab/security-tab.component.js | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/ens-ipfs/setup.js b/app/scripts/lib/ens-ipfs/setup.js index 77052b05e483..023d0d1064a3 100644 --- a/app/scripts/lib/ens-ipfs/setup.js +++ b/app/scripts/lib/ens-ipfs/setup.js @@ -58,13 +58,18 @@ export default function setupEnsIpfsResolver({ const ipfsGateway = getIpfsGateway(); const useAddressBarEnsResolution = getUseAddressBarEnsResolution(); + const ensSiteUrl = `https://app.ens.domains/name/${name}`; + /* if (!useAddressBarEnsResolution || ipfsGateway === '') { return; } + */ - browser.tabs.update(tabId, { url: `loading.html` }); + if (useAddressBarEnsResolution) { + browser.tabs.update(tabId, { url: `loading.html` }); + } - let url = `https://app.ens.domains/name/${name}`; + let url = ensSiteUrl; // If we're testing ENS domain resolution support, // we assume the ENS domains URL @@ -79,6 +84,11 @@ export default function setupEnsIpfsResolver({ name, }); if (type === 'ipfs-ns' || type === 'ipns-ns') { + // If the ENS is via IPFS and that setting is disabled, + // Do not resolve the ENS + if (ipfsGateway === '') { + return; + } const resolvedUrl = `https://${hash}.${type.slice( 0, 4, @@ -121,7 +131,9 @@ export default function setupEnsIpfsResolver({ } catch (err) { console.warn(err); } finally { - browser.tabs.update(tabId, { url }); + if (!useAddressBarEnsResolution && url === ensSiteUrl) { + browser.tabs.update(tabId, { url }); + } } } } diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js index d9b4ef76af04..749aba0903cf 100644 --- a/ui/pages/settings/security-tab/security-tab.component.js +++ b/ui/pages/settings/security-tab/security-tab.component.js @@ -408,7 +408,7 @@ export default class SecurityTab extends PureComponent { if (value) { // turning from true to false this.props.setIpfsGateway(''); - this.props.setUseAddressBarEnsResolution(false); + // this.props.setUseAddressBarEnsResolution(false); } else { // turning from false to true handleIpfsGatewayChange(this.state.ipfsGateway); From 5ce9b46d2eb8f59e4b13d6776aed830e7ac7a97e Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 20 Sep 2023 14:37:36 -0500 Subject: [PATCH 3/5] Implement conditions for ENS and IPFS settings --- app/scripts/lib/ens-ipfs/setup.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/scripts/lib/ens-ipfs/setup.js b/app/scripts/lib/ens-ipfs/setup.js index 023d0d1064a3..2ff08ef80d66 100644 --- a/app/scripts/lib/ens-ipfs/setup.js +++ b/app/scripts/lib/ens-ipfs/setup.js @@ -59,14 +59,11 @@ export default function setupEnsIpfsResolver({ const useAddressBarEnsResolution = getUseAddressBarEnsResolution(); const ensSiteUrl = `https://app.ens.domains/name/${name}`; - /* - if (!useAddressBarEnsResolution || ipfsGateway === '') { - return; - } - */ - if (useAddressBarEnsResolution) { - browser.tabs.update(tabId, { url: `loading.html` }); + // This is the only case where we can show the loading indicator + // https://github.com/MetaMask/metamask-extension/issues/20910#issuecomment-1723962822 + if (useAddressBarEnsResolution && ipfsGateway) { + browser.tabs.update(tabId, { url: 'loading.html' }); } let url = ensSiteUrl; @@ -87,6 +84,7 @@ export default function setupEnsIpfsResolver({ // If the ENS is via IPFS and that setting is disabled, // Do not resolve the ENS if (ipfsGateway === '') { + url = null; return; } const resolvedUrl = `https://${hash}.${type.slice( @@ -131,7 +129,11 @@ export default function setupEnsIpfsResolver({ } catch (err) { console.warn(err); } finally { - if (!useAddressBarEnsResolution && url === ensSiteUrl) { + if ( + url && + (useAddressBarEnsResolution || + (!useAddressBarEnsResolution && url !== ensSiteUrl)) + ) { browser.tabs.update(tabId, { url }); } } From 135ea69f37e8b815f515fbe4d62e1ecc51e8835b Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 20 Sep 2023 14:59:11 -0500 Subject: [PATCH 4/5] Remove initial fix --- ui/pages/settings/security-tab/security-tab.component.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js index 749aba0903cf..5452f7b9b4a0 100644 --- a/ui/pages/settings/security-tab/security-tab.component.js +++ b/ui/pages/settings/security-tab/security-tab.component.js @@ -408,7 +408,6 @@ export default class SecurityTab extends PureComponent { if (value) { // turning from true to false this.props.setIpfsGateway(''); - // this.props.setUseAddressBarEnsResolution(false); } else { // turning from false to true handleIpfsGatewayChange(this.state.ipfsGateway); From 8fb047196354a0ff270ba0052e59ecf1c94134f3 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 20 Sep 2023 15:49:13 -0500 Subject: [PATCH 5/5] Fix e2es --- app/scripts/lib/ens-ipfs/setup.js | 9 ++++++--- test/e2e/tests/ipfs-ens-resolution.spec.js | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/scripts/lib/ens-ipfs/setup.js b/app/scripts/lib/ens-ipfs/setup.js index 2ff08ef80d66..be997cdf1e98 100644 --- a/app/scripts/lib/ens-ipfs/setup.js +++ b/app/scripts/lib/ens-ipfs/setup.js @@ -60,8 +60,7 @@ export default function setupEnsIpfsResolver({ const ensSiteUrl = `https://app.ens.domains/name/${name}`; - // This is the only case where we can show the loading indicator - // https://github.com/MetaMask/metamask-extension/issues/20910#issuecomment-1723962822 + // We cannot show this if useAddressBarEnsResolution is off... if (useAddressBarEnsResolution && ipfsGateway) { browser.tabs.update(tabId, { url: 'loading.html' }); } @@ -71,7 +70,9 @@ export default function setupEnsIpfsResolver({ // If we're testing ENS domain resolution support, // we assume the ENS domains URL if (process.env.IN_TEST) { - browser.tabs.update(tabId, { url }); + if (useAddressBarEnsResolution || ipfsGateway) { + browser.tabs.update(tabId, { url }); + } return; } @@ -129,6 +130,8 @@ export default function setupEnsIpfsResolver({ } catch (err) { console.warn(err); } finally { + // Only forward to destination URL if a URL exists and + // useAddressBarEnsResolution is properly if ( url && (useAddressBarEnsResolution || diff --git a/test/e2e/tests/ipfs-ens-resolution.spec.js b/test/e2e/tests/ipfs-ens-resolution.spec.js index 73b2f70d66b8..ba7a1357d283 100644 --- a/test/e2e/tests/ipfs-ens-resolution.spec.js +++ b/test/e2e/tests/ipfs-ens-resolution.spec.js @@ -31,7 +31,7 @@ describe('Settings', function () { await driver.quit(); }); - it('Does not lookup IPFS data for ENS Domain when switched off', async function () { + it('Does not fetch ENS data for ENS Domain when ENS and IPFS switched off', async function () { let server; await withFixtures( @@ -54,7 +54,10 @@ describe('Settings', function () { await driver.clickElement({ text: 'Settings', tag: 'div' }); await driver.clickElement({ text: 'Security & privacy', tag: 'div' }); - // turns off IPFS domain resolution + // turns off IPFS setting + await driver.clickElement('[data-testid="ipfsToggle"] .toggle-button'); + + // turns off ENS domain resolution await driver.clickElement( '[data-testid="ipfs-gateway-resolution-container"] .toggle-button', );