From 6255979868e9c10a9490c12ead1d315c76d82b2b Mon Sep 17 00:00:00 2001
From: Howard Braham <howrad@gmail.com>
Date: Fri, 12 Jan 2024 13:46:42 -0800
Subject: [PATCH] fix: QR/Keystone notification window disappears (#22460)

## **Description**

Fixes the longstanding bug where the notification window disappears when
using a QR wallet.

## **Related issues**

Fixes: #19780

## **Manual testing steps**

1. Use any QR wallet (AirGap Vault is free)
2. Go to https://metamask.github.io/test-dapp/
3. Use any of the Sign methods (useful for testing because they do not
require gas in your account)
4. Click 'Sign' in the Notification window that appears
5. The window **does not** close
---
 ui/pages/home/home.component.js |  6 ++---
 ui/pages/home/home.container.js |  9 ++-----
 ui/selectors/selectors.js       | 44 ++-------------------------------
 ui/store/actions.ts             |  3 +++
 4 files changed, 10 insertions(+), 52 deletions(-)

diff --git a/ui/pages/home/home.component.js b/ui/pages/home/home.component.js
index d86d7f1908a1..330e932244e0 100644
--- a/ui/pages/home/home.component.js
+++ b/ui/pages/home/home.component.js
@@ -99,21 +99,21 @@ function shouldCloseNotificationPopup({
   institutionalConnectRequests,
   ///: END:ONLY_INCLUDE_IF
 }) {
-  let shouldCLose =
+  let shouldClose =
     isNotification &&
     totalUnapprovedCount === 0 &&
     !hasApprovalFlows &&
     !isSigningQRHardwareTransaction;
 
   ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
-  shouldCLose &&=
+  shouldClose &&=
     // MMI User must be shown a deeplink
     !waitForConfirmDeepLinkDialog &&
     // MMI User is connecting to custodian
     institutionalConnectRequests.length === 0;
   ///: END:ONLY_INCLUDE_IF
 
-  return shouldCLose;
+  return shouldClose;
 }
 
 export default class Home extends PureComponent {
diff --git a/ui/pages/home/home.container.js b/ui/pages/home/home.container.js
index 64fb431a7d95..615fa30101af 100644
--- a/ui/pages/home/home.container.js
+++ b/ui/pages/home/home.container.js
@@ -28,8 +28,7 @@ import {
   getShowTermsOfUse,
   getShowOutdatedBrowserWarning,
   getNewNetworkAdded,
-  hasUnsignedQRHardwareTransaction,
-  hasUnsignedQRHardwareMessage,
+  getIsSigningQRHardwareTransaction,
   getNewNftAddedMessage,
   getNewTokensImported,
   getShouldShowSeedPhraseReminder,
@@ -123,10 +122,6 @@ const mapStateToProps = (state) => {
     getWeb3ShimUsageStateForOrigin(state, originOfCurrentTab) ===
       Web3ShimUsageAlertStates.recorded;
 
-  const isSigningQRHardwareTransaction =
-    hasUnsignedQRHardwareTransaction(state) ||
-    hasUnsignedQRHardwareMessage(state);
-
   const hasWatchTokenPendingApprovals = getSuggestedTokens(state).length > 0;
 
   const hasWatchNftPendingApprovals = getSuggestedNfts(state).length > 0;
@@ -170,7 +165,7 @@ const mapStateToProps = (state) => {
       getIsBrowserDeprecated() && getShowOutdatedBrowserWarning(state),
     seedPhraseBackedUp,
     newNetworkAddedName: getNewNetworkAdded(state),
-    isSigningQRHardwareTransaction,
+    isSigningQRHardwareTransaction: getIsSigningQRHardwareTransaction(state),
     newNftAddedMessage: getNewNftAddedMessage(state),
     removeNftMessage: getRemoveNftMessage(state),
     newTokensImported: getNewTokensImported(state),
diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js
index 447ff2fa4772..309c66ba985f 100644
--- a/ui/selectors/selectors.js
+++ b/ui/selectors/selectors.js
@@ -44,7 +44,6 @@ import {
   HardwareTransportStates,
 } from '../../shared/constants/hardware-wallets';
 import { KeyringType } from '../../shared/constants/keyring';
-import { MESSAGE_TYPE } from '../../shared/constants/app';
 
 import { TRUNCATED_NAME_CHAR_LIMIT } from '../../shared/constants/labels';
 
@@ -166,47 +165,8 @@ export function getCurrentQRHardwareState(state) {
   return qrHardware || {};
 }
 
-export function hasUnsignedQRHardwareTransaction(state) {
-  const { txParams } = state.confirmTransaction.txData;
-  if (!txParams) {
-    return false;
-  }
-  const { from } = txParams;
-  const { keyrings } = state.metamask;
-  const qrKeyring = keyrings.find((kr) => kr.type === KeyringType.qr);
-  if (!qrKeyring) {
-    return false;
-  }
-  return Boolean(
-    qrKeyring.accounts.find(
-      (account) => account.toLowerCase() === from.toLowerCase(),
-    ),
-  );
-}
-
-export function hasUnsignedQRHardwareMessage(state) {
-  const { type, msgParams } = state.confirmTransaction.txData;
-  if (!type || !msgParams) {
-    return false;
-  }
-  const { from } = msgParams;
-  const { keyrings } = state.metamask;
-  const qrKeyring = keyrings.find((kr) => kr.type === KeyringType.qr);
-  if (!qrKeyring) {
-    return false;
-  }
-  switch (type) {
-    case MESSAGE_TYPE.ETH_SIGN_TYPED_DATA:
-    case MESSAGE_TYPE.ETH_SIGN:
-    case MESSAGE_TYPE.PERSONAL_SIGN:
-      return Boolean(
-        qrKeyring.accounts.find(
-          (account) => account.toLowerCase() === from.toLowerCase(),
-        ),
-      );
-    default:
-      return false;
-  }
+export function getIsSigningQRHardwareTransaction(state) {
+  return state.metamask.qrHardware?.sign?.request !== undefined;
 }
 
 export function getCurrentKeyring(state) {
diff --git a/ui/store/actions.ts b/ui/store/actions.ts
index dea9644d595a..64cc290d3558 100644
--- a/ui/store/actions.ts
+++ b/ui/store/actions.ts
@@ -42,6 +42,7 @@ import {
   hasTransactionPendingApprovals,
   getApprovalFlows,
   getCurrentNetworkTransactions,
+  getIsSigningQRHardwareTransaction,
   ///: BEGIN:ONLY_INCLUDE_IF(snaps)
   getNotifications,
   ///: END:ONLY_INCLUDE_IF
@@ -2479,6 +2480,7 @@ export function hideIpfsModal(): Action {
     type: actionConstants.SHOW_IPFS_MODAL_CLOSE,
   };
 }
+
 export function closeCurrentNotificationWindow(): ThunkAction<
   void,
   MetaMaskReduxState,
@@ -2491,6 +2493,7 @@ export function closeCurrentNotificationWindow(): ThunkAction<
     if (
       getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION &&
       !hasTransactionPendingApprovals(state) &&
+      !getIsSigningQRHardwareTransaction(state) &&
       approvalFlows.length === 0
     ) {
       closeNotificationPopup();