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

Do not check popupIsOpen on Vivaldi #9271

Merged

Conversation

PatrykLucka
Copy link
Contributor

Vivaldi is not triggering disconnect() function on port when closing the popup/notification. It is first thing being triggered after manually opening popup again. This prevents popup from being reopened (since extension thinks popup is still open).

This PR adds an exception for Vivaldi browser not to check this condition, it can trigger multiple same notifications, but it's better than no notification.

Addresses the issues brought up in #9194

@PatrykLucka PatrykLucka requested a review from a team as a code owner August 19, 2020 11:30
@PatrykLucka PatrykLucka requested a review from danjm August 19, 2020 11:30
@danjm danjm added this to the v8.1.next? milestone Nov 2, 2020
@danjm danjm self-assigned this Nov 3, 2020
@PatrykLucka PatrykLucka force-pushed the do-not-check-poupisopen-on-vivaldi branch from 6e4c28b to 0fa6e32 Compare November 3, 2020 19:59
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2020

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

This makes sense as an interim solution.

@rekmarks rekmarks merged commit d1430e7 into MetaMask:develop Nov 6, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants