From b0a105ce809b8b7e5e4431bd1ddecc523586cad0 Mon Sep 17 00:00:00 2001 From: Alexander Tseung Date: Mon, 16 Apr 2018 23:03:47 -0700 Subject: [PATCH] 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 ++- old-ui/app/conf-tx.js | 5 +- ui/app/components/modals/modal.js | 11 ++-- ui/app/components/pages/unlock.js | 5 +- ui/app/first-time/init-menu.js | 5 +- ui/app/reducers/metamask.js | 5 +- ui/app/unlock.js | 5 +- 12 files changed, 104 insertions(+), 68 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 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() } diff --git a/old-ui/app/conf-tx.js b/old-ui/app/conf-tx.js index 67eb31c37..dc1dc0538 100644 --- a/old-ui/app/conf-tx.js +++ b/old-ui/app/conf-tx.js @@ -6,8 +6,9 @@ const actions = require('../../ui/app/actions') const NetworkIndicator = require('./components/network') const LoadingIndicator = require('./components/loading') const txHelper = require('../lib/tx-helper') -const isPopupOrNotification = require('../../app/scripts/lib/is-popup-or-notification') const log = require('loglevel') +const { ENVIRONMENT_TYPE_NOTIFICATION } = require('../../app/scripts/lib/enums') +const { getEnvironmentType } = require('../../app/scripts/lib/util') const PendingTx = require('./components/pending-tx') const PendingMsg = require('./components/pending-msg') @@ -51,7 +52,7 @@ ConfirmTxScreen.prototype.render = function () { var txData = unconfTxList[props.index] || {} var txParams = txData.params || {} - var isNotification = isPopupOrNotification() === 'notification' + var isNotification = getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION log.info(`rendering a combined ${unconfTxList.length} unconf msg & txs`) if (unconfTxList.length === 0) return h(Loading, { isLoading: true }) diff --git a/ui/app/components/modals/modal.js b/ui/app/components/modals/modal.js index 9250cc77e..43dcd20ae 100644 --- a/ui/app/components/modals/modal.js +++ b/ui/app/components/modals/modal.js @@ -5,7 +5,8 @@ const connect = require('react-redux').connect const FadeModal = require('boron').FadeModal const actions = require('../../actions') const isMobileView = require('../../../lib/is-mobile-view') -const isPopupOrNotification = require('../../../../app/scripts/lib/is-popup-or-notification') +const { getEnvironmentType } = require('../../../../app/scripts/lib/util') +const { ENVIRONMENT_TYPE_POPUP } = require('../../../../app/scripts/lib/enums') // Modal Components const BuyOptions = require('./buy-options-modal') @@ -162,7 +163,7 @@ const MODALS = { ], mobileModalStyle: { width: '95%', - top: isPopupOrNotification() === 'popup' ? '52vh' : '36.5vh', + top: getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP ? '52vh' : '36.5vh', }, laptopModalStyle: { width: '449px', @@ -179,7 +180,7 @@ const MODALS = { ], mobileModalStyle: { width: '95%', - top: isPopupOrNotification() === 'popup' ? '52vh' : '36.5vh', + top: getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP ? '52vh' : '36.5vh', }, laptopModalStyle: { width: '449px', @@ -196,7 +197,7 @@ const MODALS = { ], mobileModalStyle: { width: '95%', - top: isPopupOrNotification() === 'popup' ? '52vh' : '36.5vh', + top: getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP ? '52vh' : '36.5vh', }, laptopModalStyle: { width: '449px', @@ -208,7 +209,7 @@ const MODALS = { contents: h(ConfirmResetAccount), mobileModalStyle: { width: '95%', - top: isPopupOrNotification() === 'popup' ? '52vh' : '36.5vh', + top: getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP ? '52vh' : '36.5vh', }, laptopModalStyle: { width: '473px', diff --git a/ui/app/components/pages/unlock.js b/ui/app/components/pages/unlock.js index b3320da21..567b72518 100644 --- a/ui/app/components/pages/unlock.js +++ b/ui/app/components/pages/unlock.js @@ -11,7 +11,8 @@ const { setNetworkEndpoints, setFeatureFlag, } = require('../../actions') -const environmentType = require('../../../../app/scripts/lib/environment-type') +const { ENVIRONMENT_TYPE_POPUP } = require('../../../../app/scripts/lib/enums') +const { getEnvironmentType } = require('../../../../app/scripts/lib/util') const getCaretCoordinates = require('textarea-caret') const EventEmitter = require('events').EventEmitter const Mascot = require('../mascot') @@ -131,7 +132,7 @@ class UnlockScreen extends Component { this.props.markPasswordForgotten() this.props.history.push(RESTORE_VAULT_ROUTE) - if (environmentType() === 'popup') { + if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP) { global.platform.openExtensionInBrowser() } }, diff --git a/ui/app/first-time/init-menu.js b/ui/app/first-time/init-menu.js index 692b01c8c..3df040922 100644 --- a/ui/app/first-time/init-menu.js +++ b/ui/app/first-time/init-menu.js @@ -8,7 +8,8 @@ const actions = require('../actions') const Tooltip = require('../components/tooltip') const getCaretCoordinates = require('textarea-caret') const { RESTORE_VAULT_ROUTE, DEFAULT_ROUTE } = require('../routes') -const environmentType = require('../../../app/scripts/lib/environment-type') +const { getEnvironmentType } = require('../../../app/scripts/lib/util') +const { ENVIRONMENT_TYPE_POPUP } = require('../../../app/scripts/lib/enums') const { OLD_UI_NETWORK_TYPE } = require('../../../app/scripts/config').enums class InitializeMenuScreen extends Component { @@ -180,7 +181,7 @@ class InitializeMenuScreen extends Component { showRestoreVault () { this.props.markPasswordForgotten() - if (environmentType() === 'popup') { + if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP) { global.platform.openExtensionInBrowser() } diff --git a/ui/app/reducers/metamask.js b/ui/app/reducers/metamask.js index e65dc2d11..5f965fbe0 100644 --- a/ui/app/reducers/metamask.js +++ b/ui/app/reducers/metamask.js @@ -1,7 +1,8 @@ const extend = require('xtend') const actions = require('../actions') const MetamascaraPlatform = require('../../../app/scripts/platforms/window') -const environmentType = require('../../../app/scripts/lib/environment-type') +const { getEnvironmentType } = require('../../../app/scripts/lib/util') +const { ENVIRONMENT_TYPE_POPUP } = require('../../../app/scripts/lib/enums') const { OLD_UI_NETWORK_TYPE } = require('../../../app/scripts/config').enums module.exports = reduceMetamask @@ -15,7 +16,7 @@ function reduceMetamask (state, action) { isUnlocked: false, isAccountMenuOpen: false, isMascara: window.platform instanceof MetamascaraPlatform, - isPopup: environmentType() === 'popup', + isPopup: getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP, rpcTarget: 'https://rawtestrpc.metamask.io/', identities: {}, unapprovedTxs: {}, diff --git a/ui/app/unlock.js b/ui/app/unlock.js index 84d8b7e7c..1325656af 100644 --- a/ui/app/unlock.js +++ b/ui/app/unlock.js @@ -7,7 +7,8 @@ const actions = require('./actions') const getCaretCoordinates = require('textarea-caret') const EventEmitter = require('events').EventEmitter const { OLD_UI_NETWORK_TYPE } = require('../../app/scripts/config').enums -const environmentType = require('../../app/scripts/lib/environment-type') +const { getEnvironmentType } = require('../../app/scripts/lib/util') +const { ENVIRONMENT_TYPE_POPUP } = require('../../app/scripts/lib/enums') const Mascot = require('./components/mascot') @@ -77,7 +78,7 @@ UnlockScreen.prototype.render = function () { h('p.pointer', { onClick: () => { this.props.dispatch(actions.markPasswordForgotten()) - if (environmentType() === 'popup') { + if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_POPUP) { global.platform.openExtensionInBrowser() } },