-
Notifications
You must be signed in to change notification settings - Fork 325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(mv3): Manifest V3 Migration Checklist #1170
Changes from 7 commits
4dee9c6
54e9edb
937e0f7
2de3028
98622b7
274f7be
f5dcefd
b05e120
6819356
36b7f74
0f99265
80dcb0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
v18.14.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"browser_action": { | ||
"action": { | ||
"browser_style": false | ||
}, | ||
"options_ui": { | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,29 +71,13 @@ export function createContextMenus ( | |
getState, _runtime, ipfsPathValidator, { onAddFromContext, onCopyRawCid, onCopyAddressAtPublicGw }) { | ||
try { | ||
const createSubmenu = (id, contextType, menuBuilder) => { | ||
browser.contextMenus.create({ | ||
id, | ||
title: browser.i18n.getMessage(id), | ||
documentUrlPatterns: ['<all_urls>'], | ||
contexts: [contextType] | ||
}) | ||
browser.contextMenus.onClicked.addListener((...args) => console.log(args)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we still need this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be gone. |
||
} | ||
const createImportToIpfsMenuItem = (parentId, id, contextType, ipfsAddOptions) => { | ||
const itemId = `${parentId}_${id}` | ||
apiMenuItems.add(itemId) | ||
return browser.contextMenus.create({ | ||
id: itemId, | ||
parentId, | ||
title: browser.i18n.getMessage(id), | ||
contexts: [contextType], | ||
documentUrlPatterns: ['<all_urls>'], | ||
enabled: false, | ||
/* no support for 'icons' in Chrome | ||
icons: { | ||
'48': '/ui-kit/icons/stroke_cube.svg' | ||
}, */ | ||
onclick: (context) => onAddFromContext(context, contextType, ipfsAddOptions) | ||
}) | ||
return browser.contextMenus.onClicked.addListener((context) => onAddFromContext(context, contextType, ipfsAddOptions) | ||
) | ||
} | ||
const createCopierMenuItem = (parentId, id, contextType, handler) => { | ||
const itemId = `${parentId}_${id}` | ||
|
@@ -102,22 +86,9 @@ export function createContextMenus ( | |
if (apiMenuItemIds.has(id)) { | ||
apiMenuItems.add(itemId) | ||
} | ||
return browser.contextMenus.create({ | ||
id: itemId, | ||
parentId, | ||
title: browser.i18n.getMessage(id), | ||
contexts: [contextType], | ||
documentUrlPatterns: [ | ||
'*://*/ipfs/*', '*://*/ipns/*', | ||
'*://*.ipfs.dweb.link/*', '*://*.ipns.dweb.link/*', // TODO: add any custom public gateway from Preferences | ||
'*://*.ipfs.localhost/*', '*://*.ipns.localhost/*' | ||
], | ||
/* no support for 'icons' in Chrome | ||
icons: { | ||
'48': '/ui-kit/icons/stroke_copy.svg' | ||
}, */ | ||
onclick: (context) => handler(context, contextType) | ||
}) | ||
return browser.contextMenus.onClicked.addListener( | ||
(context) => handler(context, contextType) | ||
) | ||
} | ||
const buildSubmenu = (parentId, contextType) => { | ||
createSubmenu(parentId, contextType) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,6 @@ import createRuntimeChecks from './runtime-checks.js' | |
import { createContextMenus, findValueForContext, contextMenuCopyAddressAtPublicGw, contextMenuCopyRawCid, contextMenuCopyCanonicalAddress, contextMenuViewOnGateway, contextMenuCopyPermalink, contextMenuCopyCidAddress } from './context-menus.js' | ||
import { registerSubdomainProxy } from './http-proxy.js' | ||
import { runPendingOnInstallTasks } from './on-installed.js' | ||
import { handleConsentFromState, startSession, endSession, trackView } from './telemetry.js' | ||
const log = debug('ipfs-companion:main') | ||
log.error = debug('ipfs-companion:main:error') | ||
|
||
|
@@ -57,11 +56,8 @@ export default async function init () { | |
runtime = await createRuntimeChecks(browser) | ||
state = initState(options) | ||
notify = createNotifier(getState) | ||
// ensure consent is set properly on app init | ||
handleConsentFromState(state) | ||
|
||
if (state.active) { | ||
startSession() | ||
// It's ok for this to fail, node might be unavailable or mis-configured | ||
try { | ||
ipfs = await initIpfsClient(browser, state) | ||
|
@@ -108,7 +104,7 @@ export default async function init () { | |
throw new Error('IPFS Companion: API client is disabled') | ||
} | ||
|
||
function registerListeners () { | ||
function registerListeners() { | ||
const onBeforeSendInfoSpec = ['blocking', 'requestHeaders'] | ||
if (browser.webRequest.OnBeforeSendHeadersOptions && 'EXTRA_HEADERS' in browser.webRequest.OnBeforeSendHeadersOptions) { | ||
// Chrome 72+ requires 'extraHeaders' for accessing all headers | ||
|
@@ -172,16 +168,6 @@ export default async function init () { | |
const result = validIpfsOrIpns(path) ? resolveToPublicUrl(path) : null | ||
return Promise.resolve({ pubGwUrlForIpfsOrIpnsPath: result }) | ||
} | ||
if (request.telemetry) { | ||
return Promise.resolve(onTelemetryMessage(request.telemetry, sender)) | ||
} | ||
} | ||
|
||
function onTelemetryMessage (request, sender) { | ||
if (request.trackView) { | ||
const { version } = browser.runtime.getManifest() | ||
return trackView(request.trackView, { version }) | ||
} | ||
} | ||
|
||
// PORTS (connection-based messaging) | ||
|
@@ -473,7 +459,7 @@ export default async function init () { | |
// ------------------------------------------------------------------- | ||
|
||
async function updateBrowserActionBadge () { | ||
if (typeof browser.browserAction.setBadgeBackgroundColor === 'undefined') { | ||
if (typeof browser.action.setBadgeBackgroundColor === 'undefined') { | ||
// Firefox for Android does not have this UI, so we just skip it | ||
return | ||
} | ||
Comment on lines
+466
to
469
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is supported in firefox for android now, https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/action/setBadgeBackgroundColor#browser_compatibility, so we can remove the comment, but we may still want to double check for the existence of this method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the weird part is, firefox for android limited the number of extensions, there was IIRC a brief moment in time to get this running, but not anymore for the foreseeable future. I replaced a regex, so could be removed, I'd rather not touch it rn. |
||
|
@@ -498,13 +484,13 @@ export default async function init () { | |
badgeIcon = '/icons/ipfs-logo-off.svg' | ||
} | ||
try { | ||
const oldColor = colorArraytoHex(await browser.browserAction.getBadgeBackgroundColor({})) | ||
const oldColor = colorArraytoHex(await browser.action.getBadgeBackgroundColor({})) | ||
if (badgeColor !== oldColor) { | ||
await browser.browserAction.setBadgeBackgroundColor({ color: badgeColor }) | ||
await browser.action.setBadgeBackgroundColor({ color: badgeColor }) | ||
await setBrowserActionIcon(badgeIcon) | ||
} | ||
const oldText = await browser.browserAction.getBadgeText({}) | ||
if (oldText !== badgeText) await browser.browserAction.setBadgeText({ text: badgeText }) | ||
const oldText = await browser.action.getBadgeText({}) | ||
if (oldText !== badgeText) await browser.action.setBadgeText({ text: badgeText }) | ||
} catch (error) { | ||
console.error('Unable to update browserAction badge due to error', error) | ||
} | ||
|
@@ -525,14 +511,17 @@ export default async function init () { | |
let iconDefinition = { path: iconPath } | ||
try { | ||
// Try SVG first -- Firefox supports it natively | ||
await browser.browserAction.setIcon(iconDefinition) | ||
await browser.action.setIcon(iconDefinition) | ||
if (browser.runtime.lastError.message === 'Icon invalid.') { | ||
throw new Error('Icon invalid.') | ||
whizzzkid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} catch (error) { | ||
// Fallback! | ||
// Chromium does not support SVG [ticket below is 8 years old, I can't even..] | ||
// https://bugs.chromium.org/p/chromium/issues/detail?id=29683 | ||
// Still, we want icon, so we precompute rasters of popular sizes and use them instead | ||
iconDefinition = await rasterIconDefinition(iconPath) | ||
await browser.browserAction.setIcon(iconDefinition) | ||
await browser.action.setIcon(iconDefinition) | ||
} | ||
} | ||
|
||
|
@@ -568,8 +557,6 @@ export default async function init () { | |
await registerSubdomainProxy(getState, runtime) | ||
shouldRestartIpfsClient = true | ||
shouldStopIpfsClient = !state.active | ||
// Any time the extension switches active state, start or stop the current session. | ||
state.active ? startSession() : endSession() | ||
break | ||
case 'ipfsNodeType': | ||
if (change.oldValue !== braveNodeType && change.newValue === braveNodeType) { | ||
|
@@ -636,8 +623,6 @@ export default async function init () { | |
break | ||
} | ||
} | ||
// ensure consent is set properly on state changes | ||
handleConsentFromState(state) | ||
|
||
if ((state.active && shouldRestartIpfsClient) || shouldStopIpfsClient) { | ||
try { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import { browserActionFilesCpImportCurrentTab } from '../../lib/ipfs-import.js' | |
import { ipfsContentPath } from '../../lib/ipfs-path.js' | ||
import { welcomePage, optionsPage } from '../../lib/constants.js' | ||
import { contextMenuViewOnGateway, contextMenuCopyAddressAtPublicGw, contextMenuCopyPermalink, contextMenuCopyRawCid, contextMenuCopyCanonicalAddress, contextMenuCopyCidAddress } from '../../lib/context-menus.js' | ||
import { endSession, handleConsentFromState, startSession, trackView } from '../../lib/telemetry.js' | ||
|
||
// The store contains and mutates the state for the app | ||
export default (state, emitter) => { | ||
|
@@ -38,7 +39,8 @@ export default (state, emitter) => { | |
let port | ||
|
||
emitter.on('DOMContentLoaded', async () => { | ||
browser.runtime.sendMessage({ telemetry: { trackView: 'browser-action' } }) | ||
handleConsentFromState(state) | ||
trackView('browser-action') | ||
|
||
// initial render with status stub | ||
emitter.emit('render') | ||
|
@@ -205,6 +207,7 @@ export default (state, emitter) => { | |
const prev = state.active | ||
state.active = !prev | ||
if (!state.active) { | ||
endSession() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we're only calling startSession and endSession inside this DOMContentLoaded for browser-action now, then I don't think we're going to be tracking sessions properly anymore. Is that blocked by ipfs-shipyard/ignite-metrics#103 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm.. that can be revisited, this was to just make sure we didn't miss out on these events, but these or on the best effort basis. |
||
state.gatewayAddress = state.pubGwURLString | ||
state.ipfsApiUrl = null | ||
state.gatewayVersion = null | ||
|
@@ -213,6 +216,8 @@ export default (state, emitter) => { | |
} | ||
try { | ||
await browser.storage.local.set({ active: state.active }) | ||
startSession() | ||
handleConsentFromState(state) | ||
} catch (error) { | ||
console.error(`Unable to update global Active flag due to ${error}`) | ||
state.active = prev | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at https://developer.chrome.com/docs/extensions/mv3/manifest/web_accessible_resources/ and the first line
It appears that this is needed so that webpages can test if companion is being used, and if so, they can load recovery.html/js/css. Is that correct? are there other usecases where this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are resources needed by the extension to load a webpage from the extension.
They changed the way, we don't need
<all_urls>
fixing it.