From b0a105ce809b8b7e5e4431bd1ddecc523586cad0 Mon Sep 17 00:00:00 2001 From: Alexander Tseung Date: Mon, 16 Apr 2018 23:03:47 -0700 Subject: Fix confirmation popup not always opening --- app/scripts/background.js | 66 ++++++++++++++++++++--------- app/scripts/lib/enums.js | 9 ++++ app/scripts/lib/environment-type.js | 10 ----- app/scripts/lib/is-popup-or-notification.js | 12 ------ app/scripts/lib/util.js | 32 ++++++++++---- app/scripts/ui.js | 7 +-- 6 files changed, 83 insertions(+), 53 deletions(-) create mode 100644 app/scripts/lib/enums.js delete mode 100644 app/scripts/lib/environment-type.js delete mode 100644 app/scripts/lib/is-popup-or-notification.js (limited to 'app/scripts') diff --git a/app/scripts/background.js b/app/scripts/background.js index 35c484ec5..6550e8944 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -21,6 +21,11 @@ const setupMetamaskMeshMetrics = require('./lib/setupMetamaskMeshMetrics') const EdgeEncryptor = require('./edge-encryptor') const getFirstPreferredLangCode = require('./lib/get-first-preferred-lang-code') const getObjStructure = require('./lib/getObjStructure') +const { + ENVIRONMENT_TYPE_POPUP, + ENVIRONMENT_TYPE_NOTIFICATION, + ENVIRONMENT_TYPE_FULLSCREEN, +} = require('./lib/enums') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = process.env.METAMASK_DEBUG @@ -43,7 +48,7 @@ const isEdge = !isIE && !!window.StyleMedia let popupIsOpen = false let notificationIsOpen = false -let openMetamaskTabsIDs = {} +const openMetamaskTabsIDs = {} // state persistence const diskStore = new LocalStorageStore({ storageKey: STORAGE_KEY }) @@ -172,7 +177,7 @@ function setupController (initState, initLangCode) { return versionedData } - function persistData(state) { + function persistData (state) { if (!state) { throw new Error('MetaMask - updated state is missing', state) } @@ -191,33 +196,53 @@ function setupController (initState, initLangCode) { // // connect to other contexts // - extension.runtime.onConnect.addListener(connectRemote) + + const metamaskInternalProcessHash = { + [ENVIRONMENT_TYPE_POPUP]: true, + [ENVIRONMENT_TYPE_NOTIFICATION]: true, + [ENVIRONMENT_TYPE_FULLSCREEN]: true, + } + + const isClientOpenStatus = () => { + return popupIsOpen || Boolean(Object.keys(openMetamaskTabsIDs).length) || notificationIsOpen + } + function connectRemote (remotePort) { - const isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' + const processName = remotePort.name + const isMetaMaskInternalProcess = metamaskInternalProcessHash[processName] const portStream = new PortStream(remotePort) + if (isMetaMaskInternalProcess) { // communication with popup - popupIsOpen = popupIsOpen || (remotePort.name === 'popup') controller.isClientOpen = true controller.setupTrustedCommunication(portStream, 'MetaMask') - // record popup as closed - if (remotePort.sender.url.match(/home.html$/)) { - openMetamaskTabsIDs[remotePort.sender.tab.id] = true - } - if (remotePort.name === 'popup') { + + if (processName === ENVIRONMENT_TYPE_POPUP) { + popupIsOpen = true + endOfStream(portStream, () => { popupIsOpen = false - if (remotePort.sender.url.match(/home.html$/)) { - openMetamaskTabsIDs[remotePort.sender.tab.id] = false - } - controller.isClientOpen = popupIsOpen || - Object.keys(openMetamaskTabsIDs).some(key => openMetamaskTabsIDs[key]) + controller.isClientOpen = isClientOpenStatus() }) } - if (remotePort.name === 'notification') { + + if (processName === ENVIRONMENT_TYPE_NOTIFICATION) { + notificationIsOpen = true + endOfStream(portStream, () => { notificationIsOpen = false + controller.isClientOpen = isClientOpenStatus() + }) + } + + if (processName === ENVIRONMENT_TYPE_FULLSCREEN) { + const tabId = remotePort.sender.tab.id + openMetamaskTabsIDs[tabId] = true + + endOfStream(portStream, () => { + delete openMetamaskTabsIDs[tabId] + controller.isClientOpen = isClientOpenStatus() }) } } else { @@ -260,10 +285,11 @@ function setupController (initState, initLangCode) { // popup trigger function triggerUi () { - extension.tabs.query({ active: true }, (tabs) => { - const currentlyActiveMetamaskTab = tabs.find(tab => openMetamaskTabsIDs[tab.id]) - if (!popupIsOpen && !currentlyActiveMetamaskTab && !notificationIsOpen) notificationManager.showPopup() - notificationIsOpen = true + extension.tabs.query({ active: true }, tabs => { + const currentlyActiveMetamaskTab = Boolean(tabs.find(tab => openMetamaskTabsIDs[tab.id])) + if (!popupIsOpen && !currentlyActiveMetamaskTab && !notificationIsOpen) { + notificationManager.showPopup() + } }) } diff --git a/app/scripts/lib/enums.js b/app/scripts/lib/enums.js new file mode 100644 index 000000000..0a3afca47 --- /dev/null +++ b/app/scripts/lib/enums.js @@ -0,0 +1,9 @@ +const ENVIRONMENT_TYPE_POPUP = 'popup' +const ENVIRONMENT_TYPE_NOTIFICATION = 'notification' +const ENVIRONMENT_TYPE_FULLSCREEN = 'fullscreen' + +module.exports = { + ENVIRONMENT_TYPE_POPUP, + ENVIRONMENT_TYPE_NOTIFICATION, + ENVIRONMENT_TYPE_FULLSCREEN, +} diff --git a/app/scripts/lib/environment-type.js b/app/scripts/lib/environment-type.js deleted file mode 100644 index 7966926eb..000000000 --- a/app/scripts/lib/environment-type.js +++ /dev/null @@ -1,10 +0,0 @@ -module.exports = function environmentType () { - const url = window.location.href - if (url.match(/popup.html$/)) { - return 'popup' - } else if (url.match(/home.html$/)) { - return 'responsive' - } else { - return 'notification' - } -} diff --git a/app/scripts/lib/is-popup-or-notification.js b/app/scripts/lib/is-popup-or-notification.js deleted file mode 100644 index ad3e825c0..000000000 --- a/app/scripts/lib/is-popup-or-notification.js +++ /dev/null @@ -1,12 +0,0 @@ -module.exports = function isPopupOrNotification () { - const url = window.location.href - // if (url.match(/popup.html$/) || url.match(/home.html$/)) { - // Below regexes needed for feature toggles (e.g. see line ~340 in ui/app/app.js) - // Revert below regexes to above commented out regexes before merge to master - if (url.match(/popup.html(?:\?.+)*$/) || - url.match(/home.html(?:\?.+)*$/) || url.match(/home.html(?:#.*)*$/)) { - return 'popup' - } else { - return 'notification' - } -} diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index 6dee9edf0..df815906f 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -1,20 +1,27 @@ const ethUtil = require('ethereumjs-util') const assert = require('assert') const BN = require('bn.js') - -module.exports = { - getStack, - sufficientBalance, - hexToBn, - bnToHex, - BnMultiplyByFraction, -} +const { + ENVIRONMENT_TYPE_POPUP, + ENVIRONMENT_TYPE_NOTIFICATION, + ENVIRONMENT_TYPE_FULLSCREEN, +} = require('./enums') function getStack () { const stack = new Error('Stack trace generator - not an error').stack return stack } +const getEnvironmentType = (url = window.location.href) => { + if (url.match(/popup.html(?:\?.+)*$/)) { + return ENVIRONMENT_TYPE_POPUP + } else if (url.match(/home.html(?:\?.+)*$/) || url.match(/home.html(?:#.*)*$/)) { + return ENVIRONMENT_TYPE_FULLSCREEN + } else { + return ENVIRONMENT_TYPE_NOTIFICATION + } +} + function sufficientBalance (txParams, hexBalance) { // validate hexBalance is a hex string assert.equal(typeof hexBalance, 'string', 'sufficientBalance - hexBalance is not a hex string') @@ -42,3 +49,12 @@ function BnMultiplyByFraction (targetBN, numerator, denominator) { const denomBN = new BN(denominator) return targetBN.mul(numBN).div(denomBN) } + +module.exports = { + getStack, + getEnvironmentType, + sufficientBalance, + hexToBn, + bnToHex, + BnMultiplyByFraction, +} diff --git a/app/scripts/ui.js b/app/scripts/ui.js index c326ca1c3..bdab29c1e 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -3,7 +3,8 @@ const OldMetaMaskUiCss = require('../../old-ui/css') const NewMetaMaskUiCss = require('../../ui/css') const startPopup = require('./popup-core') const PortStream = require('./lib/port-stream.js') -const isPopupOrNotification = require('./lib/is-popup-or-notification') +const { getEnvironmentType } = require('./lib/util') +const { ENVIRONMENT_TYPE_NOTIFICATION } = require('./lib/enums') const extension = require('extensionizer') const ExtensionPlatform = require('./platforms/extension') const NotificationManager = require('./lib/notification-manager') @@ -27,7 +28,7 @@ async function start() { // injectCss(css) // identify window type (popup, notification) - const windowType = isPopupOrNotification() + const windowType = getEnvironmentType(window.location.href) global.METAMASK_UI_TYPE = windowType closePopupIfOpen(windowType) @@ -69,7 +70,7 @@ async function start() { function closePopupIfOpen (windowType) { - if (windowType !== 'notification') { + if (windowType !== ENVIRONMENT_TYPE_NOTIFICATION) { // should close only chrome popup notificationManager.closePopup() } -- cgit