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.

Sign up for GitHub

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

Jump to bottom

MM is stuck once click out of swap pop up - UI issue #11216

Closed
angelcheung22 opened this issue Sep 16, 2024 · 2 comments
Closed

MM is stuck once click out of swap pop up - UI issue #11216

angelcheung22 opened this issue Sep 16, 2024 · 2 comments
Assignees
@dawnseeker8
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking team-hardware-wallets

Comments

@angelcheung22
Copy link

in the case of uniswap in the browser, if I click outside the popup it goes to the Ledger website in the background and if I swipe the popup away I’m stuck on the Ledger website there’s no way to change tab, go back or anything, you don’t even see the MetaMask controls anymore, the only way to get unstuck is to hard shut MetaMask.
@angelcheung22 angelcheung22 added Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild team-hardware-wallets labels Sep 16, 2024
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Sep 16, 2024
@vivek-consensys vivek-consensys added Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking and removed Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild labels Oct 9, 2024
@vivek-consensys
Copy link
Contributor

MM also crashing when user rejects the txn on the device, when swapping.

Private Zenhub Video

@dawnseeker8 dawnseeker8 mentioned this issue Oct 14, 2024
Merged
7 tasks
@vivek-consensys
Copy link
Contributor

Tested on bitrise build

Working as expected without any app crashes when clicking out of the swap pop up.

Private Zenhub Video

github-merge-queue bot pushed a commit that referenced this issue Oct 18, 2024
@dawnseeker8
fix: Fix the mobile crash due to ledger bluetooth relative exception (#…
6d6c420 
…11769)

<!--
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 PR will fix the mobile crash issue when user disable the blind
signing and reject the transaction in ledger devices.
This PR create a global exception handler to handle the bluetooth
relative exception throw directly from native code, which can't be
handled by our javascript code, (useLedgerBluetooth.ts has code handle
those exception but shomehow, the exception still bubble up to
`ExceptionHandler` in react native core which cause the red screen and
app crash).

The global exception will specially handle the Ledger relative exception
and stop exception bubble to react native core to stop app crash.

<!--
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?
-->

## **Related issues**

Fixes: #11166 
#11216 

## **Manual testing steps**

1. Have Ledger with Eth app 1.11.2 installed and have eth app settings
on Ledger set to blind signing: disabled
2. Connect to [test dapp](https://metamask.github.io/test-dapp/) on MM
mobile browser
3. Scroll down to ERC1155 section and tap "deploy"
4. Note "Confirm transaction on your Ledger" shown on mobile client
5. Note error "Unexpected error occurred. An unexpected error occurred.
Please try again"
6. Note MM closes on android and presents crash error on iOS
7. 
## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

https://api.zenhub.com/attachedFiles/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBenN6Qnc9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--8d54ab43db845febde47fa78240c0aff20dbbff9/RPReplay_Final1728461873.MP4


![image](https://github.com/user-attachments/assets/bee32826-6b44-4016-94fb-d54624c992c5)

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dawnseeker8 dawnseeker8

Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking team-hardware-wallets
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dawnseeker8 @metamaskbot @angelcheung22 @vivek-consensys