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

Reject "connect" request when user closes popup #6489

Closed
marsrobertson opened this issue Apr 22, 2019 · 6 comments
Closed

Reject "connect" request when user closes popup #6489

marsrobertson opened this issue Apr 22, 2019 · 6 comments
Assignees
Labels
area-UI Relating to the user interface.
Milestone

Comments

@marsrobertson
Copy link

marsrobertson commented Apr 22, 2019

Mod edit: when a user closes an eip1102 "Connect" request, we can provide an error/callback message to the dapp (as we now do with the transaction/confirm window)


Demo: https://youtu.be/oy6zN9NM_r4

try {
  accounts = await ethereum.enable()
} catch (error) {
  console.log(error)
}

Closing the window should give some callback message.

Otherwise some sloppy UI may wait for callback that never comes.

@bdresser bdresser changed the title Better handling of closing MetaMask popup Reject "connect" request when user closes popup Apr 23, 2019
@bdresser
Copy link
Contributor

Handled for regular tx in #3124

As stated in #3124 (comment) this can apply to the 1102 Connect request as well

Previously filed as #5700

@bitpshr @danjm any concerns?

@marsrobertson
Copy link
Author

You have much better searching skills than me, I was expecting this is likely to cause some outcry, feel free to mark as duplicate.

#5700 (comment):

This means that when a user closes an approval request rather than responding to it, it's the same as if they minimized it: the Promise will continue to hang, waiting for either an explicit approval or rejection.

This is bad.

I was thinking about some timeout long-polling to as a workaround to this bug.

Nope - let the website if the window was closed - the intention of the user was clear - closing window means no.

Minimizing window... That could trigger some event too.

if(metamask minimised) then 
     1. hide spinner
     2. display notice "metamask minimised"

@danjm danjm added area-UI Relating to the user interface. reserved labels Jun 21, 2019
@danjm danjm removed the reserved label Aug 26, 2019
@jtrein
Copy link

jtrein commented Sep 18, 2019

@marsrobertson Did you find an interim solution to this? I was also thinking about long-polling, but reluctant.

@jtrein
Copy link

jtrein commented Sep 18, 2019

I did notice that when the popup is triggered open, the event web3Instance.currentProvider.publicConfigStore.on("update", callback) will emit every ~20000ms. If it stops emitting, then we can assume (most likely unsafely) that the popup window has been closed by the user.

@tmashuang
Copy link
Contributor

Reopening issue, the linked PR doesn't seem to implement Reject on window close.

@tmashuang tmashuang reopened this Nov 12, 2019
@Gudahtt
Copy link
Member

Gudahtt commented Nov 25, 2019

This was fixed by #7401

@Gudahtt Gudahtt closed this as completed Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-UI Relating to the user interface.
Projects
None yet
Development

No branches or pull requests

7 participants