From e2b2fe3699ac5f6c5f47b0c2fc020222b52f1711 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 26 Feb 2019 14:37:03 +0100 Subject: [PATCH] refactor: isRedirectPageActionsContext Added ipfsPathValidator.isRedirectPageActionsContext with tests: Per-site toggle won't be shown on internal pages and non-IPNS urls loaded from local gateway (confusing to users) Removed redundant variables in Browser Action context and made UI less jittery. --- add-on/src/lib/ipfs-companion.js | 8 ++-- add-on/src/lib/ipfs-path.js | 11 +++++ .../popup/browser-action/context-actions.js | 12 ++--- .../popup/browser-action/gateway-status.js | 6 +-- add-on/src/popup/browser-action/header.js | 4 +- add-on/src/popup/browser-action/store.js | 48 ++++++++----------- test/functional/lib/ipfs-path.test.js | 47 +++++++++++++++--- 7 files changed, 86 insertions(+), 50 deletions(-) diff --git a/add-on/src/lib/ipfs-companion.js b/add-on/src/lib/ipfs-companion.js index 083e0a652..8617d4304 100644 --- a/add-on/src/lib/ipfs-companion.js +++ b/add-on/src/lib/ipfs-companion.js @@ -235,10 +235,12 @@ module.exports = async function init () { info.gatewayVersion = null } if (info.currentTab) { - info.ipfsPageActionsContext = ipfsPathValidator.isIpfsPageActionsContext(info.currentTab.url) - info.currentDnslinkFqdn = dnslinkResolver.findDNSLinkHostname(info.currentTab.url) - info.currentFqdn = info.currentDnslinkFqdn || new URL(info.currentTab.url).hostname + const url = info.currentTab.url + info.isIpfsContext = ipfsPathValidator.isIpfsPageActionsContext(url) + info.currentDnslinkFqdn = dnslinkResolver.findDNSLinkHostname(url) + info.currentFqdn = info.currentDnslinkFqdn || new URL(url).hostname info.currentTabRedirectOptOut = info.noRedirectHostnames && info.noRedirectHostnames.includes(info.currentFqdn) + info.isRedirectContext = info.currentFqdn && ipfsPathValidator.isRedirectPageActionsContext(url) } // Still here? if (browserActionPort) { diff --git a/add-on/src/lib/ipfs-path.js b/add-on/src/lib/ipfs-path.js index f8acc0d32..e74395fd6 100644 --- a/add-on/src/lib/ipfs-path.js +++ b/add-on/src/lib/ipfs-path.js @@ -67,6 +67,17 @@ function createIpfsPathValidator (getState, dnsLink) { // TODO: include hostname check for DNSLink and display option to copy CID even if no redirect isIpfsPageActionsContext (url) { return (IsIpfs.url(url) && !url.startsWith(getState().apiURLString)) || IsIpfs.subdomain(url) + }, + + // Test if actions such as 'per site redirect toggle' should be enabled for the URL + isRedirectPageActionsContext (url) { + const state = getState() + return state.active && // show only when actionable + state.ipfsNodeType === 'external' && // hide with embedded node + (IsIpfs.ipnsUrl(url) || // show on /ipns/ + (url.startsWith('http') && // hide on non-HTTP pages + !url.startsWith(state.gwURLString) && // hide on /ipfs/* + !url.startsWith(state.apiURLString))) // hide on api port } } diff --git a/add-on/src/popup/browser-action/context-actions.js b/add-on/src/popup/browser-action/context-actions.js index b860a94a0..2a664ca1f 100644 --- a/add-on/src/popup/browser-action/context-actions.js +++ b/add-on/src/popup/browser-action/context-actions.js @@ -9,7 +9,8 @@ const { contextMenuCopyAddressAtPublicGw, contextMenuCopyRawCid, contextMenuCopy // Context Actions are displayed in Browser Action and Page Action (FF only) function contextActions ({ active, - globalRedirectEnabled, + redirect, + isRedirectContext, currentFqdn, currentTabRedirectOptOut, ipfsNodeType, @@ -25,7 +26,6 @@ function contextActions ({ onUnPin }) { const activePinControls = active && isIpfsOnline && isApiAvailable && !(isPinning || isUnPinning) - const activeSiteRedirectSwitch = active && globalRedirectEnabled && ipfsNodeType === 'external' const renderIpfsContextItems = () => { if (!isIpfsContext) return @@ -62,9 +62,9 @@ function contextActions ({ } const renderSiteRedirectToggle = () => { - if (!activeSiteRedirectSwitch) return + if (!isRedirectContext) return const siteRedirectToggleLabel = browser.i18n.getMessage( - globalRedirectEnabled && !currentTabRedirectOptOut + active && redirect && !currentTabRedirectOptOut ? 'panel_activeTabSiteRedirectDisable' : 'panel_activeTabSiteRedirectEnable', currentFqdn @@ -74,7 +74,7 @@ function contextActions ({ text: siteRedirectToggleLabel, title: siteRedirectToggleLabel, addClass: 'truncate', - disabled: !activeSiteRedirectSwitch, + disabled: !(active && redirect), onClick: onToggleSiteRedirect })} ` @@ -92,7 +92,7 @@ module.exports.contextActions = contextActions // "Active Tab" section is displayed in Browser Action only // if redirect can be toggled or current tab has any IPFS Context Actions function activeTabActions (state) { - const showActiveTabSection = (state.active && state.globalRedirectEnabled && state.ipfsNodeType === 'external') || state.isIpfsContext + const showActiveTabSection = (state.redirect && state.isRedirectContext) || state.isIpfsContext if (!showActiveTabSection) return return html`
diff --git a/add-on/src/popup/browser-action/gateway-status.js b/add-on/src/popup/browser-action/gateway-status.js index bbe8dd411..57815f7f7 100644 --- a/add-on/src/popup/browser-action/gateway-status.js +++ b/add-on/src/popup/browser-action/gateway-status.js @@ -18,15 +18,11 @@ function statusEntry ({ label, labelLegend, value, check, itemClass = '', valueC } module.exports = function gatewayStatus ({ - active, - onToggleActive, ipfsApiUrl, gatewayAddress, gatewayVersion, swarmPeers, - isIpfsOnline, - ipfsNodeType, - globalRedirectEnabled + ipfsNodeType }) { const api = ipfsApiUrl && ipfsNodeType === 'embedded' ? 'js-ipfs' : ipfsApiUrl return html` diff --git a/add-on/src/popup/browser-action/header.js b/add-on/src/popup/browser-action/header.js index 69f5b56e1..9d069c3ae 100644 --- a/add-on/src/popup/browser-action/header.js +++ b/add-on/src/popup/browser-action/header.js @@ -10,7 +10,7 @@ const optionsIcon = require('./options-icon') const gatewayStatus = require('./gateway-status') module.exports = function header (props) { - const { ipfsNodeType, active, globalRedirectEnabled, onToggleActive, onToggleGlobalRedirect, onOpenPrefs, isIpfsOnline } = props + const { ipfsNodeType, active, redirect, onToggleActive, onToggleGlobalRedirect, onOpenPrefs, isIpfsOnline } = props const showGlobalRedirectSwitch = ipfsNodeType === 'external' return html`
@@ -31,7 +31,7 @@ module.exports = function header (props) { title: 'panel_headerActiveToggleTitle', action: onToggleActive })} - ${showGlobalRedirectSwitch ? redirectIcon({ active: active && globalRedirectEnabled, + ${showGlobalRedirectSwitch ? redirectIcon({ active: active && redirect, title: 'panel_headerRedirectToggleTitle', action: onToggleGlobalRedirect }) : null} diff --git a/add-on/src/popup/browser-action/store.js b/add-on/src/popup/browser-action/store.js index e38ca367e..e3b8be1da 100644 --- a/add-on/src/popup/browser-action/store.js +++ b/add-on/src/popup/browser-action/store.js @@ -9,17 +9,16 @@ const { contextMenuCopyAddressAtPublicGw, contextMenuCopyRawCid, contextMenuCopy // The store contains and mutates the state for the app module.exports = (state, emitter) => { Object.assign(state, { - // Global ON/OFF + // Global toggles active: true, - // UI state - isIpfsContext: false, + redirect: true, + // UI contexts + isIpfsContext: false, // Active Tab represents IPFS resource + isRedirectContext: false, // Active Tab or its subresources could be redirected isPinning: false, isUnPinning: false, isPinned: false, - currentTab: null, - currentFqdn: null, - currentDnslinkFqdn: null, - // IPFS status + // IPFS details ipfsNodeType: 'external', isIpfsOnline: false, ipfsApiUrl: null, @@ -27,10 +26,12 @@ module.exports = (state, emitter) => { gatewayAddress: null, swarmPeers: null, gatewayVersion: null, - noRedirectHostnames: [], - globalRedirectEnabled: false, - currentTabRedirectOptOut: false, - isApiAvailable: false + isApiAvailable: false, + // isRedirectContext + currentTab: null, + currentFqdn: null, + currentDnslinkFqdn: null, + noRedirectHostnames: [] }) let port @@ -42,17 +43,16 @@ module.exports = (state, emitter) => { port = browser.runtime.connect({ name: 'browser-action-port' }) port.onMessage.addListener(async (message) => { if (message.statusUpdate) { - let status = message.statusUpdate + const status = message.statusUpdate console.log('In browser action, received message from background:', message) await updateBrowserActionState(status) emitter.emit('render') - if (status.ipfsPageActionsContext) { + if (status.isIpfsContext) { // calculating pageActions states is expensive (especially pin-related checks) // we update them in separate step to keep UI snappy await updatePageActionsState(status) emitter.emit('render') } - console.log('statusAfterUpdate', status) } }) // fix for https://github.com/ipfs-shipyard/ipfs-companion/issues/318 @@ -149,7 +149,7 @@ module.exports = (state, emitter) => { }) emitter.on('toggleGlobalRedirect', async () => { - const redirectEnabled = state.globalRedirectEnabled + const redirectEnabled = state.redirect // If all integrations were suspended.. if (!state.active) { // ..clicking on 'inactive' toggle implies user wants to go online @@ -157,17 +157,16 @@ module.exports = (state, emitter) => { // if redirect was already on, then we dont want to disable it, as it would be bad UX if (redirectEnabled) return } - state.globalRedirectEnabled = !redirectEnabled - state.gatewayAddress = state.globalRedirectEnabled ? state.gwURLString : state.pubGwURLString + state.redirect = !redirectEnabled + state.gatewayAddress = state.redirect ? state.gwURLString : state.pubGwURLString emitter.emit('render') try { await browser.storage.local.set({ useCustomGateway: !redirectEnabled }) } catch (error) { console.error(`Unable to update redirect state due to ${error}`) - state.globalRedirectEnabled = redirectEnabled + state.redirect = redirectEnabled } - emitter.emit('render') }) @@ -231,8 +230,8 @@ module.exports = (state, emitter) => { state.gatewayVersion = null state.swarmPeers = null state.isIpfsOnline = false + state.isRedirectContext = false } - emitter.emit('render') try { await browser.storage.local.set({ active: state.active }) } catch (error) { @@ -243,12 +242,6 @@ module.exports = (state, emitter) => { }) async function updatePageActionsState (status) { - // Check if current page is an IPFS one - state.isIpfsContext = status.ipfsPageActionsContext || false - state.currentTab = status.currentTab || null - state.currentFqdn = status.currentFqdn || null - state.currentTabRedirectOptOut = state.noRedirectHostnames.includes(state.currentFqdn) - // browser.pageAction-specific items that can be rendered earlier (snappy UI) requestAnimationFrame(async () => { const tabId = state.currentTab ? { tabId: state.currentTab.id } : null @@ -280,20 +273,19 @@ module.exports = (state, emitter) => { } else { state.gatewayAddress = status.pubGwURLString } - state.globalRedirectEnabled = state.active && status.redirect // Upload requires access to the background page (https://github.com/ipfs-shipyard/ipfs-companion/issues/477) state.isApiAvailable = state.active && !!(await browser.runtime.getBackgroundPage()) && !browser.extension.inIncognitoContext // https://github.com/ipfs-shipyard/ipfs-companion/issues/243 state.swarmPeers = !state.active || status.peerCount === -1 ? null : status.peerCount state.isIpfsOnline = state.active && status.peerCount > -1 state.gatewayVersion = state.active && status.gatewayVersion ? status.gatewayVersion : null state.ipfsApiUrl = state.active ? status.apiURLString : null - state.webuiRootUrl = status.webuiRootUrl } else { state.ipfsNodeType = 'external' state.swarmPeers = null state.isIpfsOnline = false state.gatewayVersion = null state.isIpfsContext = false + state.isRedirectContext = false } } diff --git a/test/functional/lib/ipfs-path.test.js b/test/functional/lib/ipfs-path.test.js index b249644a9..641cd9215 100644 --- a/test/functional/lib/ipfs-path.test.js +++ b/test/functional/lib/ipfs-path.test.js @@ -100,23 +100,58 @@ describe('ipfs-path.js', function () { }) }) describe('isIpfsPageActionsContext', function () { - it('should return true for URL at IPFS Gateway', function () { - const url = 'https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest' + it('should return true for URL at a Gateway', function () { + const url = 'https://example.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest' expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(true) }) - it('should return true for URL at IPFS Gateway with Base32 CIDv1 in subdomain', function () { + it('should return true for URL at a Gateway with Base32 CIDv1 in subdomain', function () { // context-actions are shown on publick gateways that use CID in subdomain as well const url = 'http://bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy.ipfs.dweb.link/' expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(true) }) - it('should return false for URL at IPFS Gateway with Base58 CIDv0 in subdomain', function () { - // context-actions are shown on publick gateways that use CID in subdomain as well + it('should return false for URL at a Gateway with Base58 CIDv0 in subdomain', function () { + // should not be allowed, but who knows const url = 'http://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR.ipfs.dweb.link/' expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(false) }) it('should return false for non-IPFS URL', function () { - const url = 'https://ipfs.io/ipfs/NotACid?argTest#hashTest' + const url = 'https://example.com/ipfs/NotACid?argTest#hashTest' expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(false) }) }) + describe('isRedirectPageActionsContext', function () { + it('should return true for /ipfs/ URL at a Gateway', function () { + const url = 'https://example.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest' + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) + }) + it('should return true for /ipns/ URL at Local Gateway', function () { + const url = `${state.gwURL}ipns/docs.ipfs.io/?argTest#hashTest` + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) + }) + it('should return false for /ipfs/ URL at Local Gateway', function () { + const url = `${state.gwURL}/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest` + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false) + }) + it('should return false for IPFS content loaded from IPFS API port', function () { + const url = `${state.apiURL}ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest` + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false) + }) + it('should return true for URL at IPFS Gateway with Base32 CIDv1 in subdomain', function () { + // context-actions are shown on publick gateways that use CID in subdomain as well + const url = 'http://bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy.ipfs.dweb.link/' + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) + }) + it('should return true for non-IPFS HTTP URL', function () { + const url = 'https://en.wikipedia.org/wiki/Main_Page' + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) + }) + it('should return true for non-IPFS HTTPS URL', function () { + const url = 'http://en.wikipedia.org/wiki/Main_Page' + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true) + }) + it('should return false for non-HTTP URL', function () { + const url = 'moz-extension://85076b5e-900c-428f-4bf6-e6c1a33042a7/blank-page.html' + expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false) + }) + }) })