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

Fix web3 injection #1482

Merged
merged 3 commits into from
Apr 10, 2020
Merged

Fix web3 injection #1482

merged 3 commits into from
Apr 10, 2020

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Apr 9, 2020 •
edited
Loading

Summary

Fixes #1476
Fixes #1444

Some sites don't work on mobile because of the imact of the injection delay on window.web3.

On desktop, our window.web3 will be injected before the site's own scripts begin to execute. Some sites, knowing this, overwrite our window.web, because they don't want to use our version.

On mobile, this expectation is broken, and we overwrite the site's window.web3 instead. If the site's web3 is e.g. 1.2.x, its api is different from ours, 0.20.7, and things break.

This PR attempts to resolve these issues by not overwriting window.web3 if it's not falsy on page load.

Notes

@rekmarks rekmarks added type-bug Something isn't working dapp-compatibility labels Apr 9, 2020
@ibrahimtaveras00 ibrahimtaveras00 added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 9, 2020
ibrahimtaveras00
ibrahimtaveras00 approved these changes Apr 10, 2020
View reviewed changes
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Fix looks good on both OS's, QA Passed 👍

@rekmarks rekmarks marked this pull request as ready for review April 10, 2020 20:34
@rekmarks rekmarks changed the title Fix web3 injection? Fix web3 injection Apr 10, 2020
@rekmarks rekmarks mentioned this pull request Apr 10, 2020
Merged
@rekmarks rekmarks added the DO-NOT-MERGE Pull requests that should not be merged label Apr 10, 2020
@rekmarks rekmarks removed the DO-NOT-MERGE Pull requests that should not be merged label Apr 10, 2020
@rekmarks rekmarks merged commit 2eafa13 into develop Apr 10, 2020
@rekmarks rekmarks deleted the web3-fix branch April 10, 2020 21:30
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
@rekmarks
Fix web3 injection (#1482)
6b39102 
* add latest mobile-provider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibrahimtaveras00 ibrahimtaveras00 ibrahimtaveras00 approved these changes

@andrepimenta andrepimenta Awaiting requested review from andrepimenta

@danjm danjm Awaiting requested review from danjm

@estebanmino estebanmino Awaiting requested review from estebanmino

Assignees
No one assigned
Labels
dapp-compatibility needs-qa Any New Features that needs a full manual QA prior to being added to a release. type-bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Recent update breaks web3 interactions on Cent.co Trouble connecting to InstaDapp
2 participants
@rekmarks @ibrahimtaveras00