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

Can't detect new built-in "crypto wallets" extension of brave browser in javascript #7166

Open
ppeinsold opened this issue Dec 2, 2019 · 2 comments

Comments

@ppeinsold
Copy link

@ppeinsold ppeinsold commented Dec 2, 2019

Description

In javascript there is no "window.ethereum" or "window.web3" available if only crypto wallets is enabled and metamask is not installed. Do you maybe inject a different variable? As long as the MetaMask extension is installed the window.ethereum property is existing and otherwise not. I've also checked if the native wallet of Brave and its API is documented somewhere but I sadly couldn't find any documentation about it.

Steps to Reproduce

Let me add a little bit of a code example. The following code is to connect to web3 browser using web3js library:

        // Check for injected web3 (mist/metamask)             
        // Modern dapp browsers...
        let web3Provider = null;
        if (window.ethereum) {
            web3Provider = window.ethereum;
            try {
                // Request account access
                await window.ethereum.enable();
            } catch (error) {
                // User denied account access...
                console.error("User denied account access to metamask");
                return;
            }
        }
        // Legacy dapp browsers...
        else if (window.web3) {
            web3Provider = window.web3.currentProvider;
        }
        else {
            console.error("Unable to connect to metamask");
            return;
        }

        let web3 = new Web3(web3Provider);
        ...

Actual result:

This works fine for all browsers but in brave it crashes when "crypto wallets" is enabled. The problem here is, that a check is missing. On the line where I have " console.error("Unable to connect to metamask");" I need to detect "crypto wallets" because I end up here if only "crypto wallets" is installed and metamask is not. Does anyone know how to make this check?

Expected result:

We can detect built-in "crypto wallets"-extension of brave browser with an injected variable.

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

newest brave version

@ppeinsold
Copy link
Author

@ppeinsold ppeinsold commented Dec 3, 2019

@bsclifton why do you label that as "feature". It is clearly a bug imo. We also tested it with a lot of dapps and it breaks every single one of them.

@srirambv srirambv added this to To do in Crypto Wallets via automation Dec 3, 2019
@srirambv
Copy link
Collaborator

@srirambv srirambv commented Dec 3, 2019

@ppeinsold The label is for internal use to triage the issues relating to Crypto Wallets. I have added the appropriate labels

@bbondy bbondy moved this from Untriaged to Backlog in Crypto Wallets Dec 20, 2019
@bbondy bbondy added the priority/P4 label Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Crypto Wallets
  
Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.