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

Mobile: Claim URL handling #243

Closed
Keyrxng opened this issue Jun 8, 2024 · 11 comments · Fixed by #245
Closed

Mobile: Claim URL handling #243

Keyrxng opened this issue Jun 8, 2024 · 11 comments · Fixed by #245

Comments

@Keyrxng
Copy link
Member

Keyrxng commented Jun 8, 2024

The claim string can break the app on mobile when eth_requestAccounts is called and if a user is required to give the app permission.

The URL must be shortened and the permit must still be valid. Either process the data before the call or store the string and process it after.

time < 2 hr


I was unable to actually connect, I could select select all and add account or hardware wallet but couldn't hit the buttons below those needed to actually connect.

I stuck a random connectWallet() in init.ts and visited / to get around it.

Reproduce by clearing all browsing data from MM mobile browser and then heading to a valid permit url.

image

@0x4007
Copy link
Member

0x4007 commented Jun 10, 2024

I've never seen this before. We would need to use a new encoding scheme, none of which come to mind as a quick solution. Are you suggesting to remove the query string right before the RPC method is called? If so, that should only be a line of code, right?

Insane to me how bad Metamask mobile is.

@Keyrxng
Copy link
Member Author

Keyrxng commented Jun 10, 2024

@0x4007 I was thinking more just media query and detect mobile before the eth_requestAccounts call store the permit string locally and then clear the url param with window.history or similar, fire the call and afterwards replace it the same way it was removed.

afaik this doesn't require any page reloads so it should work and already have bypassed the no claim data found in url check.

Copy link

ubiquibot bot commented Jun 11, 2024

! No price label has been set. Skipping permit generation.

Copy link

! No price label has been set. Skipping permit generation.

Copy link

ubiquibot bot commented Jun 11, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Jun 11, 2024

[ 5.9 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment15.9
Conversation Incentives
CommentFormattingRelevanceReward
I've never seen this before. We would need to use a new encoding...
5.90.795.9

[ 195.8 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueSpecification130.2
IssueTask1150
IssueComment115.6
IssueComment10
Conversation Incentives
CommentFormattingRelevanceReward
The `claim` string can break the app on mobile when `...
30.2
code:
  count: 7
  score: "7"
  words: 12
130.2
@0x4007 I was thinking more just media query and detect mobile b...
15.6
code:
  count: 2
  score: "2"
  words: 7
0.8615.6
@0x4007 I was thinking more just media query and detect mobile b...
-
code:
  count: 2
  score: "0"
  words: 7
0.86-

Copy link

[ 6.408 WXDAI ]

@0x4007
Contributions Overview
View Contribution Count Reward
Issue Comment 1 4.408
Review Comment 1 2
Conversation Incentives
Comment Formatting Relevance Reward
I've never seen this before. We would need to use a new encoding…
5.8
content:
  p:
    count: 58
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.76 4.408
Simple enough of a fix. I hope that you tested this thoroughly t…
2.5
content:
  p:
    count: 25
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.8 2

[ 173.846 WXDAI ]

@Keyrxng
Contributions Overview
View Contribution Count Reward
Issue Task 1 150
Issue Specification 1 10.56
Issue Comment 1 13.286
Review Comment 1 0
Conversation Incentives
Comment Formatting Relevance Reward
The `claim` string can break the app on mobile when `…
12.8
content:
  p:
    count: 116
    score: 1
  code:
    count: 12
    score: 1
  img:
    count: 1
    score: 0
wordValue: 0.1
formattingMultiplier: 1
0.825 10.56
@0x4007 I was thinking more just media query and detect mobile b…
14.6
content:
  p:
    count: 66
    score: 1
  code:
    count: 7
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.91 13.286
Resolves #243 You were right @0x4007 and I was over thinking it…
0
content:
  p:
    count: 29
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0
formattingMultiplier: 0
0.675 -

@Keyrxng
Copy link
Member Author

Keyrxng commented Jun 11, 2024

@0x4007 it looks like the funding wallet needs funded

@0x4007
Copy link
Member

0x4007 commented Jun 11, 2024

I need to process payroll asap. It's especially tedious at this moment because I need to work with the multisig to top up the hot wallet. Will prioritize this.

@Keyrxng
Copy link
Member Author

Keyrxng commented Jun 13, 2024

I need to process payroll asap. It's especially tedious at this moment because I need to work with the multisig to top up the hot wallet. Will prioritize this.

Do you have a rough ETA on this @0x4007?

@0x4007
Copy link
Member

0x4007 commented Jun 13, 2024

I need to process payroll asap. It's especially tedious at this moment because I need to work with the multisig to top up the hot wallet. Will prioritize this.

Do you have a rough ETA on this @0x4007?

Try now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants