Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix to avoid window location change confirmation dialog while running tests - onbeforeunload conditional registration #14169

Merged
merged 2 commits into from
Apr 9, 2018

Conversation

swmitra
Copy link
Collaborator

@swmitra swmitra commented Mar 22, 2018

This PR adds testEnvironment search param validation to skip onbeforeunload registration if the current brackets window is a test window.

@nethip @petetnt Please review.

@vickramdhawal You can use this patch to run tests 😄

@swmitra swmitra requested review from nethip and petetnt March 22, 2018 16:28
Copy link
Collaborator

@petetnt petetnt left a comment

Choose a reason for hiding this comment

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

LGTM, though I thought that #13964 would have handled this 🤔

@swmitra
Copy link
Collaborator Author

swmitra commented Mar 24, 2018

@petetnt Thanks a lot for the quick review 👍
Main problem was integration tests where the actual Brackets window navigations were giving the same problem. In order to fix all those issues we needed to differentiate between actual Brackets instances and instances launched for test.

@nethip
Copy link
Contributor

nethip commented Mar 28, 2018

Thanks for the PR @swmitra. This solves the spec runner case! Is there a possibility to handle the reload case as well?

@swmitra
Copy link
Collaborator Author

swmitra commented Mar 28, 2018

Thanks for reviewing the PR @nethip 👍
Let me try to fix the Brackets reload scenario.

@swmitra
Copy link
Collaborator Author

swmitra commented Apr 4, 2018

@nethip While trying to fix this problem in case of Brackets reload, noticed that, it can't be reproduced 😄

@nethip nethip merged commit 2d65a3c into master Apr 9, 2018
@swmitra swmitra deleted the swmitra/onUnloadFixForTestwindow branch April 9, 2018 10:17
@swmitra
Copy link
Collaborator Author

swmitra commented Apr 9, 2018

Thank you @nethip 😃

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