Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only get snap name if it's a snap dialog #21088

Merged
merged 1 commit into from Sep 28, 2023
Merged

Conversation

GuillaumeRx
Copy link
Contributor

@GuillaumeRx GuillaumeRx commented Sep 28, 2023

Description

This fixes a regression in v11.0.0 where if you would try to switch network it would crash the confirmation.

It adds a condition to only get the snap name if the confirmation is a snap dialog.

2023-09-28.11-51-25.mp4

Manual testing steps

From the issue:

  • Go to luxy.io (this is not the website that I have been working on, but from another developer with the same issue, since mine aren't deployed to production yet).
  • First login into your MetaMask extension (check that your MetaMask is the latest 11.0.0 version) and switch to one of the non-supported networks for the luxy.io website, so that after connecting you will get the switch network button (I connected to Goerli, but any testnet would work).
  • Click on the Connect button in the top right and click on the Sign button.
  • Now you should see a Network not supported button in the top right, if you don't see it, switch to some other network manually in your MetaMask. Click on the Network not supported button and select the Ethereum network button in the opened modal.
  • After clicking on the Ethereum button, MetaMask should try to switch the network.

Related issues

Fixes #21040

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained:
    • What problem this PR is solving.
    • How this problem was solved.
    • How reviewers can test my changes.
  • I’ve indicated what issue this PR is linked to: Fixes #???
  • I’ve included tests if applicable.
  • I’ve documented any added code.
  • I’ve applied the right labels on the PR (see labeling guidelines).
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@GuillaumeRx GuillaumeRx requested a review from a team as a code owner September 28, 2023 09:45
@github-actions
Copy link
Contributor

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.

@GuillaumeRx GuillaumeRx added the team-snaps DEPRECATED: Use "team-snaps-platform" or "team-snaps-ecosystem" instead label Sep 28, 2023
@metamaskbot
Copy link
Collaborator

Builds ready [441d261]
Page Load Metrics (936 ± 368 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint85126101126
domContentLoaded6812293168
load791961936767368
domInteractive6812293168
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (22fe040) 68.43% compared to head (441d261) 68.43%.
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #21088      +/-   ##
===========================================
- Coverage    68.43%   68.43%   -0.00%     
===========================================
  Files         1008     1008              
  Lines        40320    40321       +1     
  Branches     10785    10786       +1     
===========================================
  Hits         27591    27591              
- Misses       12729    12730       +1     
Files Coverage Δ
ui/pages/confirmation/confirmation.js 63.27% <33.33%> (-0.43%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GuillaumeRx GuillaumeRx merged commit 993be35 into develop Sep 28, 2023
65 checks passed
@GuillaumeRx GuillaumeRx deleted the gr/fix-snapName-crash branch September 28, 2023 11:14
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2023
@metamaskbot metamaskbot added the release-11.3.0 Issue or pull request that will be included in release 11.3.0 label Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-snaps release-11.3.0 Issue or pull request that will be included in release 11.3.0 team-snaps DEPRECATED: Use "team-snaps-platform" or "team-snaps-ecosystem" instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Invalid or no prefix found error on network switch
4 participants