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

Improve processing of redirection URL #6342

Merged
merged 1 commit into from
May 15, 2023
Merged

Improve processing of redirection URL #6342

merged 1 commit into from
May 15, 2023

Conversation

NicholasEllul
Copy link
Contributor

@NicholasEllul NicholasEllul commented May 4, 2023 •
edited

Summary

This pull request just adds an extra layer of support to ensure that we do not end up in a situation where URL processing does not occur this is done by making it absolute last thing that happens before a redirect.

Testing steps

  • Create a folder, and in that folder create a file called index.html
  • Add the following contents to that file
<html>
    <body>
        <a href="https://metamask.io">Regular Link</a>
        <a href="https://bitsofcode.eth">ENS Link</a>
    </body>
</html>
  • Run python3 -m http.server 9000 to serve this static file
  • In the MM Mobile app visit localhost:9000 and visit the page you created. Ensure both links work as expected.

@NicholasEllul NicholasEllul requested a review from a team as a code owner May 4, 2023 21:31
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

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.

@NicholasEllul NicholasEllul added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 4, 2023
sethkfman
sethkfman approved these changes May 5, 2023
View reviewed changes
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sethkfman sethkfman added No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels May 5, 2023
@sethkfman
Copy link
Contributor

@cortisiko I think this can be covered by E2E. Let me know what you think.

@sethkfman sethkfman added the release-6.7.0 6.7.0 release tickets label May 5, 2023
legobeat
legobeat approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@legobeat legobeat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't wee why not. See my comment, though.

@NicholasEllul NicholasEllul merged commit a1f2b86 into main May 15, 2023
17 checks passed
@NicholasEllul NicholasEllul deleted the ellul/update-browser-tab-url-redirection branch May 15, 2023 14:30
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2023
@cortisiko cortisiko added release-7.1.0 and removed release-6.7.0 6.7.0 release tickets labels May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sethkfman sethkfman sethkfman approved these changes

@legobeat legobeat legobeat approved these changes

Assignees
No one assigned
Labels
No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. release-7.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@NicholasEllul @sethkfman @legobeat @cortisiko