From 9101812552f89439addfa4fbd783ec4bf6b47e0e Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 18 Jul 2016 17:57:23 -0700 Subject: [PATCH 1/6] inpage - add try/catch to cleanContextForImports --- app/scripts/inpage.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index f5e54cd7b..c2b445bd1 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -53,9 +53,17 @@ var __define function cleanContextForImports () { __define = global.define - delete global.define + try { + delete global.define + } catch (_) { + console.warn('MetaMask - global.define could not be deleted.') + } } function restoreContextAfterImports () { - global.define = __define + try { + global.define = __define + } catch { + console.warn('MetaMask - global.define could not be overwritten.') + } } From b2afa16925227fbb39e30fce8f9f4bca09dec20e Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 18 Jul 2016 18:08:29 -0700 Subject: [PATCH 2/6] typo fix --- app/scripts/inpage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index c2b445bd1..055235671 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -63,7 +63,7 @@ function cleanContextForImports () { function restoreContextAfterImports () { try { global.define = __define - } catch { + } catch (_) { console.warn('MetaMask - global.define could not be overwritten.') } } From 4b8e95336d3b6dd04d7c2e6df6ee5ef449686fbc Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 20 Jul 2016 10:10:57 -0700 Subject: [PATCH 3/6] Add instructions for taking a state dump (#469) --- docs/state_dump.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 docs/state_dump.md diff --git a/docs/state_dump.md b/docs/state_dump.md new file mode 100644 index 000000000..ecb863982 --- /dev/null +++ b/docs/state_dump.md @@ -0,0 +1,15 @@ +# How to take a State Dump + +Sometimes a UI bug is hard to reproduce, but we'd like to rapidly develop against the application state that caused the bug. + +In this case, a MetaMask developer will sometimes ask a user with a bug to perform a "state dump", so we can use some internal tools to reproduce and fix the bug. + +To take a state dump, follow these steps: + +1. Get the MetaMask popup to the point where it shows the bug (the developer will probably specify exactly where). +2. Right click on the extension popup UI, and in the menu, click "Inspect". This will open the developer tools. +3. In case it isn't already selected, click the "Console" tab in the new Developer Tools window. +4. In the console, type this command exactly: `logState()`. This should print a bunch of JSON text into your console. +5. Copy that printed JSON text +6. *Optional*: Annonymize that text if you'd like (you may change all instances of an account address to another valid account address, for example) We may automate the anonymization in the future. +7. Send that JSON text to the developer, ideally pasting it in the issue regarding the bug. From 5567ea8dc5f387d971b51bd4cf46abd6e7979688 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 20 Jul 2016 10:16:18 -0700 Subject: [PATCH 4/6] Version 2.6.2 (#470) --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eed562b71..c06da79e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 2.6.2 2016-07-20 + - Fixed bug that would prevent the plugin from reopening on the first try after receiving a new transaction while locked. - Fixed bug that would render 0 ETH as a non-exact amount. diff --git a/app/manifest.json b/app/manifest.json index d1a4a2f54..2c5629f7a 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "__MSG_appName__", "short_name": "Metamask", - "version": "2.6.1", + "version": "2.6.2", "manifest_version": 2, "description": "__MSG_appDescription__", "icons": { From cdd7e40545af8e62fc586f8da120e8d05ca90653 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 20 Jul 2016 14:54:07 -0700 Subject: [PATCH 5/6] Make injected web3 fail hard on sync methods (#471) Make injected web3 fail hard on sync methods --- CHANGELOG.md | 2 ++ app/scripts/lib/inpage-provider.js | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c06da79e5..d1939570c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- MetaMask now throws descriptive errors when apps try to use synchronous web3 methods. + ## 2.6.2 2016-07-20 - Fixed bug that would prevent the plugin from reopening on the first try after receiving a new transaction while locked. diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index 3b6ec154f..e387be895 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -107,7 +107,15 @@ function createSyncProvider (providerConfig) { syncProviderUrl = MetamaskConfig.network.default } } - return new HttpProvider(syncProviderUrl) + + const provider = new HttpProvider(syncProviderUrl) + // Stubbing out the send method to throw on sync methods: + provider.send = function() { + var message = 'The MetaMask Web3 object does not support synchronous methods. See https://github.com/MetaMask/faq#all-async---think-of-metamask-as-a-light-client for details.' + throw new Error(message) + } + + return provider } function remoteStoreWithLocalStorageCache (storageKey) { From 6658bea8d444281491718f8eee7bc3ae42f91b69 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 21 Jul 2016 10:45:32 -0700 Subject: [PATCH 6/6] Implement some cross-browser practices (#473) * Add mozilla plugin key to manifest * Move all chrome references into platform-checking module Addresses #453 * Add chrome global back to linter blacklist * Add tests --- .eslintignore | 1 + .eslintrc | 1 - app/manifest.json | 5 +++ app/scripts/background.js | 7 ++-- app/scripts/chromereload.js | 3 +- app/scripts/contentscript.js | 7 ++-- app/scripts/lib/extension-instance.js | 38 +++++++++++++++++++++ app/scripts/lib/extension.js | 14 ++++++++ app/scripts/lib/notifications.js | 31 ++++++++--------- app/scripts/metamask-controller.js | 7 ++-- app/scripts/popup.js | 11 +++--- test/unit/extension-test.js | 39 ++++++++++++++++++++++ ui/app/components/transaction-list-item.js | 3 +- ui/app/info.js | 7 ++-- 14 files changed, 139 insertions(+), 35 deletions(-) create mode 100644 .eslintignore create mode 100644 app/scripts/lib/extension-instance.js create mode 100644 app/scripts/lib/extension.js create mode 100644 test/unit/extension-test.js diff --git a/.eslintignore b/.eslintignore new file mode 100644 index 000000000..df49525be --- /dev/null +++ b/.eslintignore @@ -0,0 +1 @@ +app/scripts/lib/extension-instance.js diff --git a/.eslintrc b/.eslintrc index e64b3e5be..d7cb8022e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -23,7 +23,6 @@ ], "globals": { - "chrome": true, "document": false, "navigator": false, "web3": true, diff --git a/app/manifest.json b/app/manifest.json index 2c5629f7a..4cca79c72 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -37,6 +37,11 @@ "all_frames": false } ], + "applications": { + "gecko": { + "id": "MOZILLA_EXTENSION_ID" + } + }, "permissions": [ "notifications", "storage", diff --git a/app/scripts/background.js b/app/scripts/background.js index 801dc95cf..34c994ab7 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -9,6 +9,7 @@ const createMsgNotification = require('./lib/notifications.js').createMsgNotific const messageManager = require('./lib/message-manager') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const MetamaskController = require('./metamask-controller') +const extension = require('./lib/extension') const STORAGE_KEY = 'metamask-config' @@ -65,7 +66,7 @@ function showUnconfirmedTx (txParams, txData, onTxDoneCb) { // connect to other contexts // -chrome.runtime.onConnect.addListener(connectRemote) +extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { var isMetaMaskInternalProcess = (remotePort.name === 'popup') var portStream = new PortStream(remotePort) @@ -133,8 +134,8 @@ function updateBadge (state) { if (count) { label = String(count) } - chrome.browserAction.setBadgeText({ text: label }) - chrome.browserAction.setBadgeBackgroundColor({ color: '#506F8B' }) + extension.browserAction.setBadgeText({ text: label }) + extension.browserAction.setBadgeBackgroundColor({ color: '#506F8B' }) } function loadData () { diff --git a/app/scripts/chromereload.js b/app/scripts/chromereload.js index 283a131f1..cd85a8114 100644 --- a/app/scripts/chromereload.js +++ b/app/scripts/chromereload.js @@ -25,11 +25,12 @@ // if (e.data) { // var data = JSON.parse(e.data); // if (data && data.command === 'reload') { -// chrome.runtime.reload(); +// extension.runtime.reload(); // } // } // }; +const extension = require('./lib/extension') window.LiveReloadOptions = { host: 'localhost' }; (function e (t, n, r) { function s (o, u) { if (!n[o]) { if (!t[o]) { var a = typeof require === 'function' && require; if (!u && a) return a(o, !0); if (i) return i(o, !0); var f = new Error("Cannot find module '" + o + "'"); throw f.code = 'MODULE_NOT_FOUND', f } var l = n[o] = {exports: {}}; t[o][0].call(l.exports, function (e) { var n = t[o][1][e]; return s(n ? n : e) }, l, l.exports, e, t, n, r) } return n[o].exports } var i = typeof require === 'function' && require; for (var o = 0; o < r.length; o++)s(r[o]); return s })({1: [function (require, module, exports) { diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 60b37284e..0ffe93e3c 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -1,6 +1,7 @@ const LocalMessageDuplexStream = require('./lib/local-message-stream.js') const PortStream = require('./lib/port-stream.js') const ObjectMultiplex = require('./lib/obj-multiplex') +const extension = require('./lib/extension') if (shouldInjectWeb3()) { setupInjection() @@ -10,7 +11,7 @@ if (shouldInjectWeb3()) { function setupInjection(){ // inject in-page script var scriptTag = document.createElement('script') - scriptTag.src = chrome.extension.getURL('scripts/inpage.js') + scriptTag.src = extension.extension.getURL('scripts/inpage.js') scriptTag.onload = function () { this.parentNode.removeChild(this) } var container = document.head || document.documentElement // append as first child @@ -25,7 +26,7 @@ function setupStreams(){ target: 'inpage', }) pageStream.on('error', console.error.bind(console)) - var pluginPort = chrome.runtime.connect({name: 'contentscript'}) + var pluginPort = extension.runtime.connect({name: 'contentscript'}) var pluginStream = new PortStream(pluginPort) pluginStream.on('error', console.error.bind(console)) @@ -49,4 +50,4 @@ function setupStreams(){ function shouldInjectWeb3(){ var shouldInject = (window.location.href.indexOf('.pdf') === -1) return shouldInject -} \ No newline at end of file +} diff --git a/app/scripts/lib/extension-instance.js b/app/scripts/lib/extension-instance.js new file mode 100644 index 000000000..eeab6c6d0 --- /dev/null +++ b/app/scripts/lib/extension-instance.js @@ -0,0 +1,38 @@ +const apis = [ + 'alarms', + 'bookmarks', + 'browserAction', + 'commands', + 'contextMenus', + 'cookies', + 'downloads', + 'events', + 'extension', + 'extensionTypes', + 'history', + 'i18n', + 'idle', + 'notifications', + 'pageAction', + 'runtime', + 'storage', + 'tabs', + 'webNavigation', + 'webRequest', + 'windows', +] + +function Extension () { + const _this = this + let global = window + + if (window.chrome) { + global = window.chrome + } + + apis.forEach(function (api) { + _this[api] = global[api] + }) +} + +module.exports = Extension diff --git a/app/scripts/lib/extension.js b/app/scripts/lib/extension.js new file mode 100644 index 000000000..4b670490f --- /dev/null +++ b/app/scripts/lib/extension.js @@ -0,0 +1,14 @@ +/* Extension.js + * + * A module for unifying browser differences in the WebExtension API. + * + * Initially implemented because Chrome hides all of their WebExtension API + * behind a global `chrome` variable, but we'd like to start grooming + * the code-base for cross-browser extension support. + * + * You can read more about the WebExtension API here: + * https://developer.mozilla.org/en-US/Add-ons/WebExtensions + */ + +const Extension = require('./extension-instance') +module.exports = new Extension() diff --git a/app/scripts/lib/notifications.js b/app/scripts/lib/notifications.js index b6590b0e5..6c1601df1 100644 --- a/app/scripts/lib/notifications.js +++ b/app/scripts/lib/notifications.js @@ -7,6 +7,7 @@ const h = require('react-hyperscript') const PendingTxDetails = require('../../../ui/app/components/pending-tx-details') const PendingMsgDetails = require('../../../ui/app/components/pending-msg-details') const MetaMaskUiCss = require('../../../ui/css') +const extension = require('./extension') var notificationHandlers = {} const notifications = { @@ -20,34 +21,34 @@ window.METAMASK_NOTIFIER = notifications setupListeners() function setupListeners () { - // guard for chrome bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!chrome.notifications) return console.error('Chrome notifications API missing...') + // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 + if (!extension.notifications) return console.error('Chrome notifications API missing...') // notification button press - chrome.notifications.onButtonClicked.addListener(function (notificationId, buttonIndex) { + extension.notifications.onButtonClicked.addListener(function (notificationId, buttonIndex) { var handlers = notificationHandlers[notificationId] if (buttonIndex === 0) { handlers.confirm() } else { handlers.cancel() } - chrome.notifications.clear(notificationId) + extension.notifications.clear(notificationId) }) // notification teardown - chrome.notifications.onClosed.addListener(function (notificationId) { + extension.notifications.onClosed.addListener(function (notificationId) { delete notificationHandlers[notificationId] }) } // creation helper function createUnlockRequestNotification (opts) { - // guard for chrome bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!chrome.notifications) return console.error('Chrome notifications API missing...') + // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 + if (!extension.notifications) return console.error('Chrome notifications API missing...') var message = 'An Ethereum app has requested a signature. Please unlock your account.' var id = createId() - chrome.notifications.create(id, { + extension.notifications.create(id, { type: 'basic', iconUrl: '/images/icon-128.png', title: opts.title, @@ -56,8 +57,8 @@ function createUnlockRequestNotification (opts) { } function createTxNotification (state) { - // guard for chrome bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!chrome.notifications) return console.error('Chrome notifications API missing...') + // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 + if (!extension.notifications) return console.error('Chrome notifications API missing...') renderTxNotificationSVG(state, function (err, notificationSvgSource) { if (err) throw err @@ -70,8 +71,8 @@ function createTxNotification (state) { } function createMsgNotification (state) { - // guard for chrome bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!chrome.notifications) return console.error('Chrome notifications API missing...') + // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 + if (!extension.notifications) return console.error('Chrome notifications API missing...') renderMsgNotificationSVG(state, function (err, notificationSvgSource) { if (err) throw err @@ -84,11 +85,11 @@ function createMsgNotification (state) { } function showNotification (state) { - // guard for chrome bug https://github.com/MetaMask/metamask-plugin/issues/236 - if (!chrome.notifications) return console.error('Chrome notifications API missing...') + // guard for extension bug https://github.com/MetaMask/metamask-plugin/issues/236 + if (!extension.notifications) return console.error('Chrome notifications API missing...') var id = createId() - chrome.notifications.create(id, { + extension.notifications.create(id, { type: 'image', requireInteraction: true, iconUrl: '/images/icon-128.png', diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 63970799d..17af4cc29 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6,6 +6,7 @@ const messageManager = require('./lib/message-manager') const HostStore = require('./lib/remote-store.js').HostStore const Web3 = require('web3') const ConfigManager = require('./lib/config-manager') +const extension = require('./lib/extension') module.exports = class MetamaskController { @@ -239,19 +240,19 @@ module.exports = class MetamaskController { // called from popup setRpcTarget (rpcTarget) { this.configManager.setRpcTarget(rpcTarget) - chrome.runtime.reload() + extension.runtime.reload() this.idStore.getNetwork() } setProviderType (type) { this.configManager.setProviderType(type) - chrome.runtime.reload() + extension.runtime.reload() this.idStore.getNetwork() } useEtherscanProvider () { this.configManager.useEtherscanProvider() - chrome.runtime.reload() + extension.runtime.reload() } } diff --git a/app/scripts/popup.js b/app/scripts/popup.js index 2e5b98896..20be15df7 100644 --- a/app/scripts/popup.js +++ b/app/scripts/popup.js @@ -9,6 +9,7 @@ const injectCss = require('inject-css') const PortStream = require('./lib/port-stream.js') const StreamProvider = require('web3-stream-provider') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex +const extension = require('./lib/extension') // setup app var css = MetaMaskUiCss() @@ -21,7 +22,7 @@ async.parallel({ function connectToAccountManager (cb) { // setup communication with background - var pluginPort = chrome.runtime.connect({name: 'popup'}) + var pluginPort = extension.runtime.connect({name: 'popup'}) var portStream = new PortStream(pluginPort) // setup multiplexing var mx = setupMultiplex(portStream) @@ -55,8 +56,8 @@ function setupControllerConnection (stream, cb) { function getCurrentDomain (cb) { const unknown = '' - if (!chrome.tabs) return cb(null, unknown) - chrome.tabs.query({active: true, currentWindow: true}, function (results) { + if (!extension.tabs) return cb(null, unknown) + extension.tabs.query({active: true, currentWindow: true}, function (results) { var activeTab = results[0] var currentUrl = activeTab && activeTab.url var currentDomain = url.parse(currentUrl).host @@ -68,9 +69,9 @@ function getCurrentDomain (cb) { } function clearNotifications(){ - chrome.notifications.getAll(function (object) { + extension.notifications.getAll(function (object) { for (let notification in object){ - chrome.notifications.clear(notification) + extension.notifications.clear(notification) } }) } diff --git a/test/unit/extension-test.js b/test/unit/extension-test.js new file mode 100644 index 000000000..0a03a3b97 --- /dev/null +++ b/test/unit/extension-test.js @@ -0,0 +1,39 @@ +var assert = require('assert') +var sinon = require('sinon') +const ethUtil = require('ethereumjs-util') + +var path = require('path') +var Extension = require(path.join(__dirname, '..', '..', 'app', 'scripts', 'lib', 'extension-instance.js')) + +describe('extension', function() { + + describe('with chrome global', function() { + let extension + + beforeEach(function() { + window.chrome = { + alarms: 'foo' + } + extension = new Extension() + }) + + it('should use the chrome global apis', function() { + assert.equal(extension.alarms, 'foo') + }) + }) + + describe('without chrome global', function() { + let extension + + beforeEach(function() { + window.chrome = undefined + window.alarms = 'foo' + extension = new Extension() + }) + + it('should use the global apis', function() { + assert.equal(extension.alarms, 'foo') + }) + }) + +}) diff --git a/ui/app/components/transaction-list-item.js b/ui/app/components/transaction-list-item.js index 78867fca4..796ba61ae 100644 --- a/ui/app/components/transaction-list-item.js +++ b/ui/app/components/transaction-list-item.js @@ -7,6 +7,7 @@ const addressSummary = require('../util').addressSummary const explorerLink = require('../../lib/explorer-link') const CopyButton = require('./copyButton') const vreme = new (require('vreme')) +const extension = require('../../../app/scripts/lib/extension') const TransactionIcon = require('./transaction-list-item-icon') @@ -49,7 +50,7 @@ TransactionListItem.prototype.render = function () { if (!transaction.hash || !isLinkable) return var url = explorerLink(transaction.hash, parseInt(network)) - chrome.tabs.create({ url }) + extension.tabs.create({ url }) }, style: { padding: '20px 0', diff --git a/ui/app/info.js b/ui/app/info.js index d97998fd7..4e540bd03 100644 --- a/ui/app/info.js +++ b/ui/app/info.js @@ -3,6 +3,7 @@ const Component = require('react').Component const h = require('react-hyperscript') const connect = require('react-redux').connect const actions = require('./actions') +const extension = require('../../app/scripts/lib/extension') module.exports = connect(mapStateToProps)(InfoScreen) @@ -19,7 +20,7 @@ InfoScreen.prototype.render = function () { var state = this.props var manifest try { - manifest = chrome.runtime.getManifest() + manifest = extension.runtime.getManifest() } catch (e) { manifest = { version: '2.0.0' } } @@ -105,7 +106,7 @@ InfoScreen.prototype.render = function () { h('a.info', { target: '_blank', style: { width: '85vw' }, - onClick () { chrome.tabs.create({url: 'mailto:help@metamask.io?subject=Feedback'}) }, + onClick () { extension.tabs.create({url: 'mailto:help@metamask.io?subject=Feedback'}) }, }, 'Email us any questions or comments!'), ]), @@ -124,5 +125,5 @@ InfoScreen.prototype.render = function () { } InfoScreen.prototype.navigateTo = function (url) { - chrome.tabs.create({ url }) + extension.tabs.create({ url }) }