Skip to content

Commit 82cae05

Browse files
committed
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.
1 parent 2394847 commit 82cae05

File tree

8 files changed

+86
-54
lines changed

8 files changed

+86
-54
lines changed

add-on/_locales/en/messages.json

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@
5151
"message": "Open Preferences of Browser Extension",
5252
"description": "A menu item in Browser Action pop-up (panel_openPreferences)"
5353
},
54-
"panel_globalRedirectEnable": {
55-
"message": "Enable All Redirects",
56-
"description": "A menu item in Browser Action pop-up (panel_globalRedirectEnable)"
57-
},
5854
"panel_activeTabSectionHeader": {
5955
"message": "Active Tab",
6056
"description": "A menu item in Browser Action pop-up (panel_activeTabSiteRedirectEnable)"

add-on/src/lib/ipfs-companion.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,12 @@ module.exports = async function init () {
235235
info.gatewayVersion = null
236236
}
237237
if (info.currentTab) {
238-
info.ipfsPageActionsContext = ipfsPathValidator.isIpfsPageActionsContext(info.currentTab.url)
239-
info.currentDnslinkFqdn = dnslinkResolver.findDNSLinkHostname(info.currentTab.url)
240-
info.currentFqdn = info.currentDnslinkFqdn || new URL(info.currentTab.url).hostname
238+
const url = info.currentTab.url
239+
info.isIpfsContext = ipfsPathValidator.isIpfsPageActionsContext(url)
240+
info.currentDnslinkFqdn = dnslinkResolver.findDNSLinkHostname(url)
241+
info.currentFqdn = info.currentDnslinkFqdn || new URL(url).hostname
241242
info.currentTabRedirectOptOut = info.noRedirectHostnames && info.noRedirectHostnames.includes(info.currentFqdn)
243+
info.isRedirectContext = info.currentFqdn && ipfsPathValidator.isRedirectPageActionsContext(url)
242244
}
243245
// Still here?
244246
if (browserActionPort) {

add-on/src/lib/ipfs-path.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,17 @@ function createIpfsPathValidator (getState, dnsLink) {
6767
// TODO: include hostname check for DNSLink and display option to copy CID even if no redirect
6868
isIpfsPageActionsContext (url) {
6969
return (IsIpfs.url(url) && !url.startsWith(getState().apiURLString)) || IsIpfs.subdomain(url)
70+
},
71+
72+
// Test if actions such as 'per site redirect toggle' should be enabled for the URL
73+
isRedirectPageActionsContext (url) {
74+
const state = getState()
75+
return state.active && // show only when actionable
76+
state.ipfsNodeType === 'external' && // hide with embedded node
77+
(IsIpfs.ipnsUrl(url) || // show on /ipns/<fqdn>
78+
(url.startsWith('http') && // hide on non-HTTP pages
79+
!url.startsWith(state.gwURLString) && // hide on /ipfs/*
80+
!url.startsWith(state.apiURLString))) // hide on api port
7081
}
7182
}
7283

add-on/src/popup/browser-action/context-actions.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ const { contextMenuCopyAddressAtPublicGw, contextMenuCopyRawCid, contextMenuCopy
99
// Context Actions are displayed in Browser Action and Page Action (FF only)
1010
function contextActions ({
1111
active,
12-
globalRedirectEnabled,
12+
redirect,
13+
isRedirectContext,
1314
currentFqdn,
1415
currentTabRedirectOptOut,
1516
ipfsNodeType,
@@ -25,7 +26,6 @@ function contextActions ({
2526
onUnPin
2627
}) {
2728
const activePinControls = active && isIpfsOnline && isApiAvailable && !(isPinning || isUnPinning)
28-
const activeSiteRedirectSwitch = active && globalRedirectEnabled && ipfsNodeType === 'external'
2929

3030
const renderIpfsContextItems = () => {
3131
if (!isIpfsContext) return
@@ -62,9 +62,9 @@ function contextActions ({
6262
}
6363

6464
const renderSiteRedirectToggle = () => {
65-
if (!activeSiteRedirectSwitch) return
65+
if (!isRedirectContext) return
6666
const siteRedirectToggleLabel = browser.i18n.getMessage(
67-
globalRedirectEnabled && !currentTabRedirectOptOut
67+
active && redirect && !currentTabRedirectOptOut
6868
? 'panel_activeTabSiteRedirectDisable'
6969
: 'panel_activeTabSiteRedirectEnable',
7070
currentFqdn
@@ -74,7 +74,7 @@ function contextActions ({
7474
text: siteRedirectToggleLabel,
7575
title: siteRedirectToggleLabel,
7676
addClass: 'truncate',
77-
disabled: !activeSiteRedirectSwitch,
77+
disabled: !(active && redirect),
7878
onClick: onToggleSiteRedirect
7979
})}
8080
`
@@ -92,7 +92,7 @@ module.exports.contextActions = contextActions
9292
// "Active Tab" section is displayed in Browser Action only
9393
// if redirect can be toggled or current tab has any IPFS Context Actions
9494
function activeTabActions (state) {
95-
const showActiveTabSection = (state.active && state.globalRedirectEnabled && state.ipfsNodeType === 'external') || state.isIpfsContext
95+
const showActiveTabSection = (state.redirect && state.isRedirectContext) || state.isIpfsContext
9696
if (!showActiveTabSection) return
9797
return html`
9898
<div class="no-select w-100 outline-0--focus tl">

add-on/src/popup/browser-action/gateway-status.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,11 @@ function statusEntry ({ label, labelLegend, value, check, itemClass = '', valueC
1818
}
1919

2020
module.exports = function gatewayStatus ({
21-
active,
22-
onToggleActive,
2321
ipfsApiUrl,
2422
gatewayAddress,
2523
gatewayVersion,
2624
swarmPeers,
27-
isIpfsOnline,
28-
ipfsNodeType,
29-
globalRedirectEnabled
25+
ipfsNodeType
3026
}) {
3127
const api = ipfsApiUrl && ipfsNodeType === 'embedded' ? 'js-ipfs' : ipfsApiUrl
3228
return html`

add-on/src/popup/browser-action/header.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const optionsIcon = require('./options-icon')
1010
const gatewayStatus = require('./gateway-status')
1111

1212
module.exports = function header (props) {
13-
const { ipfsNodeType, active, globalRedirectEnabled, onToggleActive, onToggleGlobalRedirect, onOpenPrefs, isIpfsOnline } = props
13+
const { ipfsNodeType, active, redirect, onToggleActive, onToggleGlobalRedirect, onOpenPrefs, isIpfsOnline } = props
1414
const showGlobalRedirectSwitch = ipfsNodeType === 'external'
1515
return html`
1616
<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;">
@@ -31,7 +31,7 @@ module.exports = function header (props) {
3131
title: 'panel_headerActiveToggleTitle',
3232
action: onToggleActive
3333
})}
34-
${showGlobalRedirectSwitch ? redirectIcon({ active: active && globalRedirectEnabled,
34+
${showGlobalRedirectSwitch ? redirectIcon({ active: active && redirect,
3535
title: 'panel_headerRedirectToggleTitle',
3636
action: onToggleGlobalRedirect
3737
}) : null}

add-on/src/popup/browser-action/store.js

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,29 @@ const { contextMenuCopyAddressAtPublicGw, contextMenuCopyRawCid, contextMenuCopy
99
// The store contains and mutates the state for the app
1010
module.exports = (state, emitter) => {
1111
Object.assign(state, {
12-
// Global ON/OFF
12+
// Global toggles
1313
active: true,
14-
// UI state
15-
isIpfsContext: false,
14+
redirect: true,
15+
// UI contexts
16+
isIpfsContext: false, // Active Tab represents IPFS resource
17+
isRedirectContext: false, // Active Tab or its subresources could be redirected
1618
isPinning: false,
1719
isUnPinning: false,
1820
isPinned: false,
19-
currentTab: null,
20-
currentFqdn: null,
21-
currentDnslinkFqdn: null,
22-
// IPFS status
21+
// IPFS details
2322
ipfsNodeType: 'external',
2423
isIpfsOnline: false,
2524
ipfsApiUrl: null,
2625
publicGatewayUrl: null,
2726
gatewayAddress: null,
2827
swarmPeers: null,
2928
gatewayVersion: null,
30-
noRedirectHostnames: [],
31-
globalRedirectEnabled: false,
32-
currentTabRedirectOptOut: false,
33-
isApiAvailable: false
29+
isApiAvailable: false,
30+
// isRedirectContext
31+
currentTab: null,
32+
currentFqdn: null,
33+
currentDnslinkFqdn: null,
34+
noRedirectHostnames: []
3435
})
3536

3637
let port
@@ -42,17 +43,16 @@ module.exports = (state, emitter) => {
4243
port = browser.runtime.connect({ name: 'browser-action-port' })
4344
port.onMessage.addListener(async (message) => {
4445
if (message.statusUpdate) {
45-
let status = message.statusUpdate
46+
const status = message.statusUpdate
4647
console.log('In browser action, received message from background:', message)
4748
await updateBrowserActionState(status)
4849
emitter.emit('render')
49-
if (status.ipfsPageActionsContext) {
50+
if (status.isIpfsContext) {
5051
// calculating pageActions states is expensive (especially pin-related checks)
5152
// we update them in separate step to keep UI snappy
5253
await updatePageActionsState(status)
5354
emitter.emit('render')
5455
}
55-
console.log('statusAfterUpdate', status)
5656
}
5757
})
5858
// fix for https://github.com/ipfs-shipyard/ipfs-companion/issues/318
@@ -149,25 +149,24 @@ module.exports = (state, emitter) => {
149149
})
150150

151151
emitter.on('toggleGlobalRedirect', async () => {
152-
const redirectEnabled = state.globalRedirectEnabled
152+
const redirectEnabled = state.redirect
153153
// If all integrations were suspended..
154154
if (!state.active) {
155155
// ..clicking on 'inactive' toggle implies user wants to go online
156156
emitter.emit('toggleActive')
157157
// if redirect was already on, then we dont want to disable it, as it would be bad UX
158158
if (redirectEnabled) return
159159
}
160-
state.globalRedirectEnabled = !redirectEnabled
161-
state.gatewayAddress = state.globalRedirectEnabled ? state.gwURLString : state.pubGwURLString
160+
state.redirect = !redirectEnabled
161+
state.gatewayAddress = state.redirect ? state.gwURLString : state.pubGwURLString
162162
emitter.emit('render')
163163

164164
try {
165165
await browser.storage.local.set({ useCustomGateway: !redirectEnabled })
166166
} catch (error) {
167167
console.error(`Unable to update redirect state due to ${error}`)
168-
state.globalRedirectEnabled = redirectEnabled
168+
state.redirect = redirectEnabled
169169
}
170-
171170
emitter.emit('render')
172171
})
173172

@@ -231,8 +230,8 @@ module.exports = (state, emitter) => {
231230
state.gatewayVersion = null
232231
state.swarmPeers = null
233232
state.isIpfsOnline = false
233+
state.isRedirectContext = false
234234
}
235-
emitter.emit('render')
236235
try {
237236
await browser.storage.local.set({ active: state.active })
238237
} catch (error) {
@@ -243,12 +242,6 @@ module.exports = (state, emitter) => {
243242
})
244243

245244
async function updatePageActionsState (status) {
246-
// Check if current page is an IPFS one
247-
state.isIpfsContext = status.ipfsPageActionsContext || false
248-
state.currentTab = status.currentTab || null
249-
state.currentFqdn = status.currentFqdn || null
250-
state.currentTabRedirectOptOut = state.noRedirectHostnames.includes(state.currentFqdn)
251-
252245
// browser.pageAction-specific items that can be rendered earlier (snappy UI)
253246
requestAnimationFrame(async () => {
254247
const tabId = state.currentTab ? { tabId: state.currentTab.id } : null
@@ -280,20 +273,19 @@ module.exports = (state, emitter) => {
280273
} else {
281274
state.gatewayAddress = status.pubGwURLString
282275
}
283-
state.globalRedirectEnabled = state.active && status.redirect
284276
// Upload requires access to the background page (https://github.com/ipfs-shipyard/ipfs-companion/issues/477)
285277
state.isApiAvailable = state.active && !!(await browser.runtime.getBackgroundPage()) && !browser.extension.inIncognitoContext // https://github.com/ipfs-shipyard/ipfs-companion/issues/243
286278
state.swarmPeers = !state.active || status.peerCount === -1 ? null : status.peerCount
287279
state.isIpfsOnline = state.active && status.peerCount > -1
288280
state.gatewayVersion = state.active && status.gatewayVersion ? status.gatewayVersion : null
289281
state.ipfsApiUrl = state.active ? status.apiURLString : null
290-
state.webuiRootUrl = status.webuiRootUrl
291282
} else {
292283
state.ipfsNodeType = 'external'
293284
state.swarmPeers = null
294285
state.isIpfsOnline = false
295286
state.gatewayVersion = null
296287
state.isIpfsContext = false
288+
state.isRedirectContext = false
297289
}
298290
}
299291

test/functional/lib/ipfs-path.test.js

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,23 +100,58 @@ describe('ipfs-path.js', function () {
100100
})
101101
})
102102
describe('isIpfsPageActionsContext', function () {
103-
it('should return true for URL at IPFS Gateway', function () {
104-
const url = 'https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
103+
it('should return true for URL at a Gateway', function () {
104+
const url = 'https://example.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
105105
expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(true)
106106
})
107-
it('should return true for URL at IPFS Gateway with Base32 CIDv1 in subdomain', function () {
107+
it('should return true for URL at a Gateway with Base32 CIDv1 in subdomain', function () {
108108
// context-actions are shown on publick gateways that use CID in subdomain as well
109109
const url = 'http://bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy.ipfs.dweb.link/'
110110
expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(true)
111111
})
112-
it('should return false for URL at IPFS Gateway with Base58 CIDv0 in subdomain', function () {
113-
// context-actions are shown on publick gateways that use CID in subdomain as well
112+
it('should return false for URL at a Gateway with Base58 CIDv0 in subdomain', function () {
113+
// should not be allowed, but who knows
114114
const url = 'http://QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR.ipfs.dweb.link/'
115115
expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(false)
116116
})
117117
it('should return false for non-IPFS URL', function () {
118-
const url = 'https://ipfs.io/ipfs/NotACid?argTest#hashTest'
118+
const url = 'https://example.com/ipfs/NotACid?argTest#hashTest'
119119
expect(ipfsPathValidator.isIpfsPageActionsContext(url)).to.equal(false)
120120
})
121121
})
122+
describe('isRedirectPageActionsContext', function () {
123+
it('should return true for /ipfs/ URL at a Gateway', function () {
124+
const url = 'https://example.com/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest'
125+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true)
126+
})
127+
it('should return true for /ipns/ URL at Local Gateway', function () {
128+
const url = `${state.gwURL}ipns/docs.ipfs.io/?argTest#hashTest`
129+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true)
130+
})
131+
it('should return false for /ipfs/ URL at Local Gateway', function () {
132+
const url = `${state.gwURL}/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest`
133+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false)
134+
})
135+
it('should return false for IPFS content loaded from IPFS API port', function () {
136+
const url = `${state.apiURL}ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest`
137+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false)
138+
})
139+
it('should return true for URL at IPFS Gateway with Base32 CIDv1 in subdomain', function () {
140+
// context-actions are shown on publick gateways that use CID in subdomain as well
141+
const url = 'http://bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy.ipfs.dweb.link/'
142+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true)
143+
})
144+
it('should return true for non-IPFS HTTP URL', function () {
145+
const url = 'https://en.wikipedia.org/wiki/Main_Page'
146+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true)
147+
})
148+
it('should return true for non-IPFS HTTPS URL', function () {
149+
const url = 'http://en.wikipedia.org/wiki/Main_Page'
150+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(true)
151+
})
152+
it('should return false for non-HTTP URL', function () {
153+
const url = 'moz-extension://85076b5e-900c-428f-4bf6-e6c1a33042a7/blank-page.html'
154+
expect(ipfsPathValidator.isRedirectPageActionsContext(url)).to.equal(false)
155+
})
156+
})
122157
})

0 commit comments

Comments
 (0)