Skip to content

Commit

Permalink
refactor: isRedirectPageActionsContext
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lidel committed Feb 26, 2019
1 parent 2394847 commit e2b2fe3
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 50 deletions.
8 changes: 5 additions & 3 deletions add-on/src/lib/ipfs-companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions add-on/src/lib/ipfs-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/<fqdn>
(url.startsWith('http') && // hide on non-HTTP pages
!url.startsWith(state.gwURLString) && // hide on /ipfs/*
!url.startsWith(state.apiURLString))) // hide on api port
}
}

Expand Down
12 changes: 6 additions & 6 deletions add-on/src/popup/browser-action/context-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -74,7 +74,7 @@ function contextActions ({
text: siteRedirectToggleLabel,
title: siteRedirectToggleLabel,
addClass: 'truncate',
disabled: !activeSiteRedirectSwitch,
disabled: !(active && redirect),
onClick: onToggleSiteRedirect
})}
`
Expand All @@ -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`
<div class="no-select w-100 outline-0--focus tl">
Expand Down
6 changes: 1 addition & 5 deletions add-on/src/popup/browser-action/gateway-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
4 changes: 2 additions & 2 deletions add-on/src/popup/browser-action/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<div class="pt3 pb1 br2 br--top ba bw1 b--white" style="background-image: url('../../../images/stars.png'), linear-gradient(to bottom, #041727 0%,#043b55 100%); background-size: 100%; background-repeat: repeat;">
Expand All @@ -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}
Expand Down
48 changes: 20 additions & 28 deletions add-on/src/popup/browser-action/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,29 @@ 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,
publicGatewayUrl: null,
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
Expand All @@ -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
Expand Down Expand Up @@ -149,25 +149,24 @@ 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
emitter.emit('toggleActive')
// 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')
})

Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}

Expand Down
47 changes: 41 additions & 6 deletions test/functional/lib/ipfs-path.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})

0 comments on commit e2b2fe3

Please sign in to comment.