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

ethereum.selectedAddress is not reset after Sign Out #7109

Closed
faudisio opened this issue Sep 5, 2019 · 5 comments · Fixed by #7161
Closed

ethereum.selectedAddress is not reset after Sign Out #7109

faudisio opened this issue Sep 5, 2019 · 5 comments · Fixed by #7161
Labels

Comments

@faudisio
Copy link

faudisio commented Sep 5, 2019

Describe the bug
If the user Signs Out from MetaMask the ethereum.selectedAddress still contains the previously selected address

To Reproduce
Steps to reproduce the behavior:

  1. Create a simple html page with a button that prints ethereum.selectedAddress to the console
  2. Sign In to MetaMask
  3. Press the button
  4. The console shows the selected address
  5. Sign Out from MetaMask
  6. Press the button
  7. The console still shows the selected address

Expected behavior
When the user does a Sign Out ethereum.selectedAddress should be reset to undefined

Browser details (please complete the following information):

  • OS: MacOS Mojave 10.14.6
  • Browsers: Chrome 76.0.3809.132 and
    Brave 0.68.133 (with Chromium 76.0.3809.132)
  • MetaMask Version 7.1.0
@Gudahtt
Copy link
Member

Gudahtt commented Sep 11, 2019

This seems related to #7101

Gudahtt added a commit that referenced this issue Sep 12, 2019
The `runtime.Port.postMessage` API will drop keys with a value of
`undefined` on Chrome, but not on Firefox. This was a problem for the
`publicConfig` stream, which passed the key `selectedAddress` with the
value of `undefined` to communicate to dapps that the user had logged
out.

Instead a `null` is now passed for `selectedAddress` upon logout, which
is correctly sent by the `runtime.Port.postMessage` API on both Chrome
and Firefox.

closes #7101
closes #7109
Gudahtt added a commit that referenced this issue Sep 13, 2019
* Replace `undefined` selectedAddress with `null`

The `runtime.Port.postMessage` API will drop keys with a value of
`undefined` on Chrome, but not on Firefox. This was a problem for the
`publicConfig` stream, which passed the key `selectedAddress` with the
value of `undefined` to communicate to dapps that the user had logged
out.

Instead a `null` is now passed for `selectedAddress` upon logout, which
is correctly sent by the `runtime.Port.postMessage` API on both Chrome
and Firefox.

closes #7101
closes #7109

* Update `metamask-inpage-provider` to v3.0.0

The v3.0.0 update includes a change to the `accountsChanged` event. The
event will now emit an empty array instead of an array with `undefined`
or `null`.

The previous behavior was to emit `[undefined]`. The previous commit
would have changed that to `[null]` anyway, so we figured if we're
going to make a public-facing change to the event anyway we should
change it to be correct. `[undefined]` was never intended, and it
technically violates EIP-1193, which states that the `accountsChanged`
event should emit an array of strings.
Gudahtt added a commit that referenced this issue Sep 13, 2019
* Replace `undefined` selectedAddress with `null`

The `runtime.Port.postMessage` API will drop keys with a value of
`undefined` on Chrome, but not on Firefox. This was a problem for the
`publicConfig` stream, which passed the key `selectedAddress` with the
value of `undefined` to communicate to dapps that the user had logged
out.

Instead a `null` is now passed for `selectedAddress` upon logout, which
is correctly sent by the `runtime.Port.postMessage` API on both Chrome
and Firefox.

closes #7101
closes #7109

* Update `metamask-inpage-provider` to v3.0.0

The v3.0.0 update includes a change to the `accountsChanged` event. The
event will now emit an empty array instead of an array with `undefined`
or `null`.

The previous behavior was to emit `[undefined]`. The previous commit
would have changed that to `[null]` anyway, so we figured if we're
going to make a public-facing change to the event anyway we should
change it to be correct. `[undefined]` was never intended, and it
technically violates EIP-1193, which states that the `accountsChanged`
event should emit an array of strings.
@tmashuang tmashuang reopened this Sep 16, 2019
@tmashuang
Copy link
Contributor

Issue still persists even after PR has been merged.

@Gudahtt
Copy link
Member

Gudahtt commented Sep 17, 2019

False alarm - it was fixed after all.

@Gudahtt Gudahtt closed this as completed Sep 17, 2019
@faudisio
Copy link
Author

Thanks all. In which version and when will the fix be released?

@Gudahtt
Copy link
Member

Gudahtt commented Sep 17, 2019

Thank you for the report! This is part of v7.2.1, which is being released this week.

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

Successfully merging a pull request may close this issue.

3 participants