Skip to content

fix: origin url displayed for signatures#13692

Merged
jpuri merged 2 commits into
release/7.41.0from
display_url_fix
Feb 24, 2025
Merged

fix: origin url displayed for signatures#13692
jpuri merged 2 commits into
release/7.41.0from
display_url_fix

Conversation

@jpuri

@jpuri jpuri commented Feb 24, 2025

Copy link
Copy Markdown
Contributor

Description

Fix request from url displayed for signatures. Wrong url is spamming sentry.

Related issues

Fixes: #13580

Manual testing steps

NA

Screenshots/Recordings

NA

Pre-merge author checklist

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**

PR fixes error thrown from DisplayURL component. I found that error
throw during rendering phase get propagated despite try/catch block. The
PR wraps code in `useEffect` hook.

## **Related issues**

Fixes: #13580

## **Manual testing steps**

1. Go to dapp not using `https://` only using `http://`
2. Submit signature
3. Check displayed url

## **Screenshots/Recordings**
TODO

## **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**

- [ ] 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.
Fix request from url displayed for signatures. Wrong url is spamming
sentry.

Fixes: #13580

NA

NA

- [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.

- [ ] 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.
@jpuri jpuri added the team-confirmations Push issues to confirmations team label Feb 24, 2025
@jpuri jpuri requested a review from a team as a code owner February 24, 2025 15:44
@github-actions

Copy link
Copy Markdown
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.

@sonarqubecloud

Copy link
Copy Markdown

@jpuri jpuri merged commit c92c9a0 into release/7.41.0 Feb 24, 2025
@jpuri jpuri deleted the display_url_fix branch February 24, 2025 16:18
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants