Version v11.16.6 #25025
Merged
Version v11.16.6 #25025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Following #23398, This PR is the second part to reorganize the test/e2e repository to create a structure that is both well-organized and easy to navigate. [](https://codespaces.new/MetaMask/metamask-extension/pull/24238?quickstart=1) ## **Related issues** Fixes: #23296 ## **Manual testing steps** Check if the organization is logical. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…code (#24326) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** The `saveTimestamp` function is called at an interval in `background.js` to prevent the service worker from dying. It is now put behind a feature flag via develop options settings page to provide an option to toggle and test. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/24326?quickstart=1) ## **Related issues** Fixes: #24224 ## **Manual testing steps** 1. run `yarn start:mv3` 2. open service worker console from extension management page 3. input `let { timestamp } = await chrome.storage.session.get('timestamp');` and then `console.log(timestamp)`, you should see latest timestamp result 4. Open extension, go to settings => developer options 5. Toggle off Service Worker Keep Alive option 6. Reload extension, repeat between step 2 - 3 7. you should see `undefined` result this time. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/MetaMask/metamask-extension/assets/12678455/6198588e-3add-4b23-ab98-80ce24a64bec **New UI:** <img width="1044" alt="截屏2024-04-30 21 53 07" src="https://github.com/MetaMask/metamask-extension/assets/12678455/452db85b-1f3a-40de-b2e2-032d06754640"> <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…tion` (#24639) ## **Description** The problem with this flaky test is that in some occurrences we are proceeding to the last Confirmation screen without the balance being loaded (race condition). This can be observed at second 4 from this video. This test fails every time this condition is met (very often in Mutlichain build, possibly due to shorter flow of UI?) The fix is simply waiting for the balance to be loaded before proceeding with the actions. However, I think we should also revise the application behaviour, as it does not seem correct that: - whenever we proceed to the last screen without the balance being loaded, the gas is set to 0 (specifically the gas limit) - then, no matter how long we wait, the gas cannot be recovered from this and we are stucked in the confirmation screen (Confirm is disabled) --> this happens despite the balance being then loaded and also getting numerous successful responses from the gas API I'll open separate tickets for that Example of failure: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/81147/workflows/ec5f9950-23fa-4f6f-b248-9f894813e75d/jobs/2874889/tests https://github.com/MetaMask/metamask-extension/assets/54408225/93b9bd5d-a9d2-4898-b743-8a59eed0eea2 [](https://codespaces.new/MetaMask/metamask-extension/pull/24639?quickstart=1) ## **Related issues** Fixes: #24574 ## **Manual testing steps** 1. Check ci jobs 2. Run test multiple times locally `yarn test:e2e:single test/e2e/tests/transaction/send-eth.spec.js --browser=chrome --leave-running=true --retry-until-failure --retries=15` ## **Screenshots/Recordings** In the test failing artifacts we can see how we cannot proceed with the transaction as the Confirm button is disabled. However the problem is that the gas is set to 0 and never recovered, but the real root cause comes from proceeding with the previous screen without the balance loaded.  ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
## **Description** "The user data directory contains profile data such as history, bookmarks, and cookies, as well as other per-installation local state." We are using the `user-data-dir` command and unnecessarily caching such data in the `test-artefacts/` directory. With local testing, and seemingly with test run as part of the same job on CI, it can be seen that this cached data has persistence between test runs, and can thereby cause flakiness. Locally, it seems that old builds of the extension can be cached. On CI, it seems that things like our fetch cache can be persisted between browser sessions. I do not believe this is actually used for any purpose, and we can reduce flakiness by removing it. [](https://codespaces.new/MetaMask/metamask-extension/pull/24696?quickstart=1) ## **Manual testing steps** all e2e tests should pass ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…tiple tokens from search' test (#24694) ## **Description** This improves on a flaky test that I noticed while working to get mv3 tests to pass. I think it is flaky on mv2 as well. The bug is not visually reproducible manually, but I saw it fail twice on CI. The test clicks "import-tokens-modal-import-button", then waits for for the loading spinner to be not present, and then finds tokens in the list. It finds the token list as a group and then checks that the expected number are present. The problem is that the list is not fully populated at the exact same time that the loading spinner is removed. The fix is to rewrite the test so that it waits for the expected number of tokens to be rendered (instead of assuming the token list is immediately in the fully populated state). [](https://codespaces.new/MetaMask/metamask-extension/pull/24694?quickstart=1) ## **Manual testing steps** This test should continue to pass on all e2e jobs ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Danica Shen <[email protected]>
… Linux + Firefox snap (#24703) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** In this PR we enable an envar which fixes the e2e for running against Firefox, when the browser is installed as a snap (Linux). For doing so, we need to leverage the Firefox [ServiceBuilder class](https://www.selenium.dev/documentation/webdriver/drivers/service/). Using the service class by default is something that we already do [in Chrome](https://github.com/MetaMask/metamask-extension/blob/develop/test/e2e/webdriver/chrome.js#L75). For Firefox, we only did this in the case a custom port was passed. Now it's done always, and in the case we pass a port, we configure it as we did. [](https://codespaces.new/MetaMask/metamask-extension/pull/24703?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Check ci FF tests continue to work normally 2. If you are on Linux, install FF as snap and run a test with the flag `FIREFOX_SNAP=true yarn test:e2e:single test/e2e/tests/tokens/nft/import-erc1155.spec.js --browser=firefox` ## **Screenshots/Recordings** ### **Before**  ### **After** https://github.com/MetaMask/metamask-extension/assets/54408225/05cdd8d4-64d7-4224-8b28-3f1dc4c2fd00 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
## **Description** This improves on a flaky test. I noticed this when working on mv3 e2e tests, although I believe it is the same problem discussed here https://github.com/MetaMask/metamask-extension/pull/24016/files#r1580648050 app-installed.spec.js expects an api request to segment. It is possible for the test to get to the point where it checks for that request before the request has been made. Decreasing the timeout delay in background.js, means the request will be called sooner, in time for the test to correctly assert. [](https://codespaces.new/MetaMask/metamask-extension/pull/24695?quickstart=1) ## **Manual testing steps** e2e tests should pass ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…er bridge (#24622) ## **Description** Fixes a bug. Personal signatures were broken for ledger on MV3. The problem was that the offscreen bridge named those message actions incorrectly. The ledger bridge expects `'ledger-sign-personal-message'`: https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/gh-pages/ledger-bridge.js#L35-L37 This is consistent with the implementation of `deviceSignMessage` on the current iframe bridge: https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/main/src/ledger-iframe-bridge.ts#L219-L226 [](https://codespaces.new/MetaMask/metamask-extension/pull/24622?quickstart=1) ## **Related issues** Fixes: #24588 ## **Manual testing steps** 1. Build MV3 2. Import a ledger 3. Go to the test dapp 4. Try a Personal Sign 5. Accept in MM 6. You should see the signature on your ledger and be able to successfully sign ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…erencesController?.enableMV3TimestampSave is false (#24697) ## **Description** This if block was added to allow us to easily toggle off the timestamp saving at an interval. (#24326) `enableMV3TimestampSave` defaults to true in the preferences controller, but on first install of the extension, before the preferences controller has been initialized, it will be `undefined`. We should ensure that this timestamp saving behaviour is able to occur on first install of the extension by only preventing it if `enableMV3TimestampSave` is explicitly `false`. [](https://codespaces.new/MetaMask/metamask-extension/pull/24697?quickstart=1) ## **Manual testing steps** Repro broken: 1. On develop, put a console log inside the `if` block that this PR modifies 2. Build and install and start to use metamask. Notice that the console log does on occur. Test fix: 1. On this branch, put a console log inside the `if` block that this PR modifies 2. Build and install and start to use metamask. Notice that the console log does occur. ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This ensures that `ENABLE_MV3=true` is correctly set for all tests run within the `test-e2e-chrome-mv3` job, and it ensures test results are stored on circleci (in the case of failures). [](https://codespaces.new/MetaMask/metamask-extension/pull/24704?quickstart=1) ## **Manual testing steps** When grouped with all the other e2e fixes, e2e tests should pass. [TODO: add list of all PRs here] ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
… document under MV3 (#24675) ## **Description** Our MV3 builds add a "Offscreen Document" to our extension. This means that when our e2e tests call `driver.getAllWindowHandles()`, a new window is included in the list (the window for the offscreen document). This PR updates the e2e tests to handle this change. Comments are left on each file to explain why the change was needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24675?quickstart=1) ## **Related issues** Fixes: #21496 ## **Manual testing steps** Any of the test files modified in this PR should pass when run locally with MV3 builds ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…s up on Token Approval transaction confirmations` (#24739) ## **Description** This PR attempts to fix the flaky test `Migrate Opensea to Blockaid Banner @no-mmi Shows up on Token Approval transaction confirmations` This test fails whenever we click Create Token for deploying an ERC20 token, as the popup is not triggered and we remain in the test dapp. I haven't been able to reproduce this failure locally. However, after looking into the code and the context of flakiness, I can confirm that: - I've seen that we are doing several unnecessary steps for the test setup, like connecting to the test dapp, or deploying an ERC20 contract. This can be achieved by fixtures, so I've added the fixtures and got rid of a good chunk of the code. The good part is that the test failure precisely happens on the test setup, which is now removed - It might be that there is a race condition between the Connect to the test Dapp and the Create Token steps, where the dapp has not yet the connected address for triggering the Create Token but the button is already enabled, resulting in the error in the test dapp `unknown account` and popup not being opened (see video below). However, those 2 steps are now removed from the test, meaning any potential race condition at this point won't happen anymore - I've added the `logInWithBalanceValidation` instead of the `unlockWallet` to make sure the balance is loaded before triggering the interaction For the above points, I have strong reasons for wanting to implement the changes on this PR, despite not being 100% sure of the above being the root cause. [](https://codespaces.new/MetaMask/metamask-extension/pull/24739?quickstart=1) ## **Related issues** Fixes: another item of this list #24603 ## **Manual testing steps** 1. Run this test locally several times `yarn test:e2e:single test/e2e/tests/ppom/migrate-opensea-to-blockaid-banner.spec.js --browser=firefox --leave-running --retryUntilFailure --retries=10 2. Check ci jobs ## **Screenshots/Recordings**   This is not the test environment, however here I demonstrate how would look like if the create token is triggered before the dapp has the connected account resolved https://github.com/MetaMask/metamask-extension/assets/54408225/1121af80-bb95-4201-86db-ff37bb99c3e7 <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fixes #24514. The problem was that if the service worker is terminated, when app-init.js runs again when the service worker is to be restarted, `importAllScripts()` is not called. The solution is just to make sure it is called in the top-level scope, on every run of the script. This should be safe to do because of the `scriptsLoadInitiated` check already in the `importAllScripts` function. The PR also uses the `serviceworker.state` property to ensure that we don't attempt to import scripts before installation. (That is explained further in the code comment added in this PR) [](https://codespaces.new/MetaMask/metamask-extension/pull/24612?quickstart=1) ## **Related issues** Fixes: #24514 ## **Manual testing steps** 1. Start MM MV3 build 2. Go to chrome://serviceworker-internals/?devtools 3. Stop Service Worker manually with the button 4. You should be able to open MetaMask ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…mage util code when creating notifications (#24631) ## **Description** MV3 bug that we needto fix: #24518 In particular, we've got: ``` LavaMoat - property "fetch" of globalThis is inaccessible under scuttling mode. at get (chrome-extension://lbnhnppncgilfagdfbanifbfolodkdaf/scripts/runtime-lavamoat.js:13190:11) at loadImageDataForServiceWorker (extensions::imageUtil:24:22) at loadImageData (extensions::imageUtil:111:5) at Object.loadAllImages (extensions::imageUtil:134:5) at replaceNotificationOptionURLs (extensions::notifications:89:13) at extensions::notifications:116:5 at chrome-extension://lbnhnppncgilfagdfbanifbfolodkdaf/common-6.js:3:431145 at new Promise (<anonymous>) at Proxy.<anonymous> (chrome-extension://lbnhnppncgilfagdfbanifbfolodkdaf/common-6.js:3:430825) at Object.apply (chrome-extension://lbnhnppncgilfagdfbanifbfolodkdaf/common-6.js:3:430244) at Object._showNotification (chrome-extension://lbnhnppncgilfagdfbanifbfolodkdaf/common-2.js:1:455026) ``` The problematic code is in not in our codebase but in chromium, here: https://github.com/chromium/chromium/blob/main/extensions/renderer/resources/image_util.js#L23-L25 You can see that function is named loadImageDataForServiceWorker and it is only used in mv3. This breaks the browser notification feature. As can be seen from the commit history and the comments in this PR, we attemtped some solutions that would allow us to provide a modified and tamed fetch to be used for this image fetching. However, there are unknown problems - related to how sentry uses fetch and how sentry is initialized - that interfere with the solutions we attempted. So for now, as discussed with @weizman, the latest commit on the PR just adds a scuttling exception for fetch (and for Offscreen Canvas, which is also needed for the browser notification feature). We will return to the root problem later, and aim to remove these scuttling exceptions. ## **Related issues** Fixes: #24518 ## **Manual testing steps** 1. `yarn dist:mv3` 2. install, onboard 3. send a transaction 4. verify that a browser notification is shown once the transaction is confirmed ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…different networks. should switch to the dapps network automatically when handling sendTransaction calls` (#24741) ## **Description** This PR fixes the flaky test `Request Queuing for Multiple Dapps and Txs on different networks. should switch to the dapps network automatically when handling sendTransaction calls `. The problem is that sometimes when we call the `const currentWindowHandles = await driver.getAllWindowHandles` there is still the previous popup open, meaning that we get the total count of 4 windows. Right after that, we trigger a send `await driver.clickElement('#sendButton');` and now we wait for the windowHandles to be `currentWindowHandles + 1` meaning we wait until we have 5 windows. However this will never happen as we should wait for 4 windows instead, but the previous windowCount was considering the previous popup, resulting in 1 more window in total. ``` const currentWindowHandles = await driver.getAllWindowHandles(); await driver.clickElement('#sendButton'); const newWindowHandles = await driver.waitUntilXWindowHandles( process.env.ENABLE_MV3 ? currentWindowHandles.length : currentWindowHandles.length + 1, ``` The proposed fix is to hardcode the expect number of windows directly, as we know how many there should be and we do this in the majority of the tests. There could be alternative approaches, - wait for a condition: though in this case the condition of having switched networks is already met in the wallet, so there doesn't seem to be another condition to wait for 🤔 - add a delay ## **Related issues** Fixes: the third item of this list #24603 ## **Manual testing steps** 1. Run the test several times `yarn test:e2e:single test/e2e/tests/request-queuing/ui.spec.js --browser=chrome --leave-running --retryUntilFailure --retries=10` 2. Check ci jobs ## **Screenshots/Recordings** See how at one point, we get current windows as 4 (we expected 3) because the popup was still not closed and the next time we trigger a tx, we would expect 4+1, which will never happen. https://github.com/MetaMask/metamask-extension/assets/54408225/a3eb7f74-cf2c-43ba-a5bb-0835544b448c ## **Pre-merge author checklist** - [X] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…onboarding user who chooses to opt out metrics` (#24789) ## **Description** The problem seems to be that sometimes Linux is not able to find the sound card (for unknown reasons). This happens specifically to any of the onboarding tests that go to the video screen. Context: other Linux users reporting this error `ALSA lib confmisc.c:855:(parse_card) cannot find card '0'` can be found [here](https://forums.debian.net/viewtopic.php?t=53516). ``` ALSA lib confmisc.c:855:(parse_card) cannot find card '0' ALSA lib conf.c:5178:(_snd_config_evaluate) function snd_func_card_inum returned error: No such file or directory ALSA lib confmisc.c:422:(snd_func_concat) error evaluating strings ALSA lib conf.c:5178:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory ALSA lib confmisc.c:1334:(snd_func_refer) error evaluating name ALSA lib conf.c:5178:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory ALSA lib conf.c:5701:(snd_config_expand) Evaluate error: No such file or directory ALSA lib pcm.c:2664:(snd_pcm_open_noupdate) Unknown PCM default ``` The solution is to not render the video Box component if we are in test mode. Circleci failures: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/82995/workflows/51b18b16-f786-4ee8-96d0-9c751af6fa17/jobs/2965494/artifacts [](https://codespaces.new/MetaMask/metamask-extension/pull/24789?quickstart=1) ## **Related issues** Fixes: #24602 ## **Manual testing steps** 1. Check ci run ## **Screenshots/Recordings** Artifacts from ci: test fails at the screen with the video  ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…ate the ui elements (#24790) ## **Description** This PR addresses a flaky test issue related to the Chain ID input field. Reason for flakiness: The root cause of the flakiness was identified as the rapid input of the entire Chain ID, which resulted in the error message appearing and persisting even after the correct Chain ID was fully entered. Observation: When entering the Chain ID manually, the error message correctly displays when incomplete values like "0x53" are input but disappears upon entering the full Chain ID with "9". To mimic more human-like actions in the automated test, I adjusted the test behavior accordingly. Key Changes: Split Chain ID Input: The Chain ID input process has been split into two distinct parts (chainId_part1 and chainId_part2). This modification adopts a more human-like approach to entering the Chain ID, allowing the error message sufficient time to respond accurately. Results: Before the Fix: The flakiness could be reproduced locally on Firefox with a 50% occurrence rate. After the Fix: After implementing the changes, I ran the test 20 consecutive times without encountering any flakiness, demonstrating the effectiveness of the fix. This adjustment ensures that the automated test behaves in a way that closely resembles user interaction, and reduce test flakiness. [](https://codespaces.new/MetaMask/metamask-extension/pull/24790?quickstart=1) ## **Related issues** Fixes: #24628 ## **Manual testing steps** 1. Run the test several times yarn test:e2e:single test/e2e/tests/request-queuing/ui.spec.js --browser=firefox --leave-running --retryUntilFailure --retries=10 2. Check ci jobs ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…fferent networks. should switch to the dapps network automatically when handling sendTransaction calls` (#24809) ## **Description** This PR fixes the flaky test `Request Queuing for Multiple Dapps and Txs on different networks. should switch to the dapps network automatically when handling sendTransaction calls`. - Circle ci failure: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/82995/workflows/51b18b16-f786-4ee8-96d0-9c751af6fa17/jobs/2965502/tests The test fails at the point of trying to find the unapproved transaction, in the transaction list. The problem is that in the test, we are doing a browser refresh in order for the transaction to show up, but there is a race condition that, if you do a refresh once the unapproved tx is already present, you are redirected to the confirmation screen. This causes that the transaction cannot be found in the activity list and the test fails. The solution is to change the approach for a more robust one, and instead of doing a browser refresh, we simply go explicitly to the activity tab and wait until the transaction appears.  **Circle ci Screenshot**: it's looking for the unapproved-transaction but it cannot find it because after reloading MetaMask, the screen is redirected to the unapproved transaction.  [](https://codespaces.new/MetaMask/metamask-extension/pull/24809?quickstart=1) ## **Related issues** Fixes: #24603 ## **Manual testing steps** 1. Check ci jobs 2. Run test multiple times `yarn test:e2e:single test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js --browser=firefox --leave-running --retryUntilFailure --retries=10ng/multiple-networks-dapps-txs` ## **Screenshots/Recordings** Behavior when you reload the browser with the unapproved transaction already present --> see how you are redirected to the confirmation page https://github.com/MetaMask/metamask-extension/assets/54408225/726911da-a044-48eb-a60f-938099db1176 Solution: no browser refresh but instead, going to the activity tab and wait until tx appears (see last seconds on the video) https://github.com/MetaMask/metamask-extension/assets/54408225/a757587d-8bfa-4ef4-acc4-78f2489daa83 <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
… locked down" (#24812) ## **Description** This PR addresses the flaky 'lockdown' e2e test, specifically when running in Firefox, which is a tricky one. Root Cause: The flakiness is caused by the rapid execution of the lockdown test script in Firefox immediately after navigating to the BACKGROUND page. This quick transition did not allow sufficient time for the environment to stabilize, preventing the script from accurately assessing the lockdown state. Fix Implemented: To mitigate this issue, I introduced a `await driver.delay(1000)` after navigating to the BACKGROUND page and before executing the lockdown test script. This brief pause ensures that the environment is fully loaded and stable, allowing the script to run under consistent conditions and accurately verify the lockdown state. While I'm agree that the use of delay should be minimized, this test's specific nature and the absence of a way to use the driver to wait for an element's status makes it necessary to use the delay() method. Results: Before the Fix: The flakiness could be reproduced locally on Firefox with approximately a 30% occurrence rate. After the Fix: After implementing the changes, I ran the test 20 consecutive times without encountering any flakiness, demonstrating the effectiveness of the fix. [](https://codespaces.new/MetaMask/metamask-extension/pull/24812?quickstart=1) ## **Related issues** Fixes: #24621 ## **Manual testing steps** 1. Run the test several times yarn test:e2e:single test/e2e/tests/request-queuing/ui.spec.js --browser=firefox --leave-running --retryUntilFailure --retries=10 2. Check ci jobs ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
… to infura before onboarding is completed/test-failure-screenshot.png` and `onboarding @no-mmi Clicks create a new wallet, accepts a secure password, reveals the Secret Recovery Phrase, confirm SRP/test-failure-screenshot.png` (#24813) ## **Description** This PR fixes more onboarding flakiness `onboarding @no-mmi doesn't make any network requests to infura before onboarding is completed/test-failure-screenshot.pn` and `onboarding @no-mmi Clicks create a new wallet, accepts a secure password, reveals the Secret Recovery Phrase, confirm SRP/test-failure-screenshot.png` . The error is originated when we try to click an element which is in stale state `StaleElementReferenceError: stale element reference: stale element not found in the current frame`. This is a race condition originated in the `clickElement` method, in the following way: 1. we try to find the clickable element 2. the element is refreshed in between 3. right after, we try to click the element from 1 --> this results in the element being stale, as the element we are trying to click is an old instance (1) instead of (2)  Extra note: this race condition is surfaced in the Onboarding tests mostly (I've only seen it fail there, so far), since I suspect that the the actions we perform there, all refresh the same react component for onboarding, giving a window for this casuistic to happen - Circle ci job failures: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/83037/workflows/b435d63d-e50c-4dcd-af7d-4f6e0856ab6c/jobs/2967823/artifacts - Circle ci logs:  [](https://codespaces.new/MetaMask/metamask-extension/pull/24813?quickstart=1) ## **Related issues** Fixes: #24602 (remaining items) ## **Manual testing steps** 1. Check ci or run onboarding tests multiple times (though I've never been able to repro this locally) ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
….driver.delay` (#24838) ## **Description** The `clickElement` method was enhanced to mitigate a race condition but mistakenly we called `this.driver.delay` instead of `this.delay`. This PR fixes the issue. Context: https://github.com/MetaMask/metamask-extension/pull/24813/files#r1617677338 Found by @Gudahtt . [](https://codespaces.new/MetaMask/metamask-extension/pull/24838?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
Flushes the memorized request ids in the duplicate request middleware every 3 minutes, and makes the middleware ignore JSON-RPC notifications. Also converts the middleware to TypeScript. The duplicate request middleware used to dedupe JSON-RPC requests with the same id under MV3 used an internal array that would grow boundlessly until the background restarted. In addition, this middleware assumed that it would never see any JSON-RPC notifications (or it would always reject the `n + 1`:th notifications, since their ids are always `undefined`). Although we in practice always add ids to JSON-RPC requests, it seems ill-advised for this middleware to make that assumption.
…24810) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** There is a racing condition captured in MV3 build. When sending transaction from dapp directly, if user chooses to edit the gas fee, and when `suggestedGasFees ` request is processing slow, `gasFeeEstimates` could be undefined within hook `useCustomTimeEstimate.js`. The fix is to add a condition for accessing this object before forwarding to UI. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1) ## **Related issues** Fixes: #24732 ## **Manual testing steps** 1. Build in MV3 2. Trigger a txn from dapp 3. Open network panel, and trigger edit gas button before `suggestedGasFees` is finished fetching 4. You should see the network edition panel ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** by @seaona https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374 <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276 - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…ce (#24874) ## **Description** Multiple onboarding tests are flaky, failing with the error `TimeoutError: Waiting for element to be located By(css selector, [data-testid="recovery-phrase-reveal"])` The problem is that the test can get to the page where that selector is present, but then be re-routed back to the previous screen. This is due to a race condition. in create-password.js there is this code: ``` useEffect(() => { if (currentKeyring) { if (firstTimeFlowType === FirstTimeFlowType.import) { ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) history.replace(ONBOARDING_COMPLETION_ROUTE); ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) history.replace(ONBOARDING_PIN_EXTENSION_ROUTE); ///: END:ONLY_INCLUDE_IF } else { ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) history.replace(ONBOARDING_SECURE_YOUR_WALLET_ROUTE); ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) history.replace(SRP_REMINDER); ///: END:ONLY_INCLUDE_IF } } }, [currentKeyring, history, firstTimeFlowType]); ``` and there is this also this code inside handleCreate(): ``` } else { // Otherwise we are in create new wallet flow try { if (createNewAccount) { await createNewAccount(password); } ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) history.push(ONBOARDING_SECURE_YOUR_WALLET_ROUTE); ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) history.replace(SRP_REMINDER); ///: END:ONLY_INCLUDE_IF } catch (error) { setPasswordError(error.message); } ``` What is happening is: 1. user clicks submit/confirm on the create password screen 2. await createNewAccount(password); occurs 3. before that fully resolves, at least a new keyring is created, so the useEffect fires and history.replace(ONBOARDING_SECURE_YOUR_WALLET_ROUTE); so the user/test is taken to the next route and continues with the clicks on the "Secure your wallet" and then "Write down your Secret Recovery Phrase" screen 4. then `await createNewAccount(password);` resolves, and the user/test is rerouted to the "Secure Your Wallet" screen again The fix is to prevent the `history.replace(ONBOARDING_SECURE_YOUR_WALLET_ROUTE)` call in the `useEffect` if `handleCreate` has already started to create a new account [](https://codespaces.new/MetaMask/metamask-extension/pull/24874?quickstart=1) ## **Related issues** Fixes: #24608 ## **Manual testing steps** e2e tests should pass ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
This was referenced Jun 4, 2024
Cherry picks [98385ff](98385ff) (#25051) to Version-v11.16.6 Co-authored-by: Frederik Bolding <[email protected]>
This was referenced Jun 5, 2024
Builds ready [d3895c2]
Page Load Metrics (78 ± 16 ms)
|
Builds ready [155b20c]
Page Load Metrics (205 ± 205 ms)
|
|
MV3 changes were tested and validated by the extension platform team One the latest commit I tested the mv3 flask build against a few dapps and snaps I have also tested hardware wallets, and version upgrade scenarios, on this branch. |
](https://codespaces.new/MetaMask/metamask-extension/pull/24238?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24326?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24639?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24696?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24694?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24703?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24695?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24622?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24697?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24704?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24675?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24739?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24612?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24789?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24790?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24809?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24812?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24813?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24838?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/24874?quickstart=1){kind=link}
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.