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.

Sign up for GitHub

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

Jump to bottom

beforeunload when running Electron #796

Closed
paulpruteanu opened this issue Oct 23, 2017 · 11 comments
Closed

beforeunload when running Electron #796

paulpruteanu opened this issue Oct 23, 2017 · 11 comments
Labels
type: duplicate This issue or pull request already exists

Comments

@paulpruteanu
Copy link

paulpruteanu commented Oct 23, 2017 •
edited by jennifer-shehane

Current behavior:

When I run a test suite on a page that has a beforeunload event listener - hence, it triggers an alert -, first test goes fine, but the second test onwards fails to open the page.

This works fine using Chrome, as the Debugger console shows the following:

Blocked attempt to show a 'beforeunload' confirmation panel for a frame that never had a user gesture since its load. https://www.chromestatus.com/feature/5082396709879808

This makes CI headless tests unfeasible, as they fail for no consistent reason.

Desired behavior:

Electron built-in headless browser should prohibit beforeunload behaviour - same as Chrome does.

How to reproduce:

Open a page that has

window.addEventListener("beforeunload", function (e) {
    (e || window.event).returnValue = 'Are you sure you want to close this page?';
})

in two tests within the same test suite.

Test code:

index.html # started with any node server of your preference

<html>
<head>
    <title>
        Testing
    </title>
    <script>
        window.addEventListener("beforeunload", function (e) {
            (e || window.event).returnValue = 'Are you sure you want to close this page?';
        })
    </script>
</head>
<body>
<div id="helloWorld">Hello World!</div>
<button onClick="document.getElementById('helloWorld').style.display = 'none';">Click me!</button>
</body>
</html>
describe(`failing electron test suite`, () => {

        beforeEach(() => {
            cy.visit('http://127.0.0.1:8080/index.html')
        })
        it('sees hello and clicks to hide', () => {
            cy.contains('Hello').should('be.visible')
            cy.get('button').click();
        })

        it('should see world opening the page again', () => {
            cy.contains('World').should('be.visible')
        })

})

Additional Info (images, stack traces, etc)

I managed to fix this by exposing the listener in window, and run

cy.window()
    .then(win => win.myListener && win.removeEventListener('beforeunload', win.myListener))

straight after

cy.visit('<url>')
  • Operating System: MacOS Sierra 10.12.6 (16G29)
  • Cypress Version: 0.20.1
  • Failing Browser Version: Electron 53
  • Working Browser Version: Chrome 61.0.3163.100
@brian-mann
Copy link
Member

I'm confused why this behavior causes tests in Electron to fail. What about your test code requires this dialog? Can you post your test code?

Electron isn't showing the dialog because its using Chromium 53, and you're likely using a new version of Chrome which has had this feature implemented.

You could also avoid adding this event listener entirely under Cypress tests.

if (window.Cypress) {
  ...
}

@paulpruteanu
Copy link
Author

I've updated the comment, it's happening whenever you change something within DOM.

@brian-mann
Copy link
Member

Okay thank you - this should be enough for us to reproduce.

@brian-mann
Copy link
Member

Also BTW you could run Chrome in CI by using the --browser chrome option. You'll need it locally installed but we provide docker images with it prebuilt.

@paulpruteanu
Copy link
Author

Good to know. Regarding window.Cypress, I'd rather leave the code agnostic to testing frameworks, and I can live with a global signature of the listener, that I can control in the testing environment.

@brian-mann
Copy link
Member

That's fair. At the very least you could conditionally make the event listener global only in the Cypress test environment.

There is potentially a different approach here where you use the unload event listeners from within Cypress itself and then you remove the e.returnValue which then would prevent that error from occurring.

In that situation you could test that the returnValue was set correctly (programmatically) but remove it to prevent the browser from reacting to it - since you don't care about it anyway.

https://docs.cypress.io/api/events/catalog-of-events.html#

@jennifer-shehane jennifer-shehane added stage: needs investigating Someone from Cypress needs to look at this type: bug labels Oct 24, 2017
@paulpruteanu paulpruteanu mentioned this issue Nov 7, 2017
Closed
@jennifer-shehane jennifer-shehane added this to the 2.0.0 milestone Feb 8, 2018
@brian-mann
Copy link
Member

brian-mann commented Feb 15, 2018 •
edited

It appears this feature was implemented in Chrome 60.

I think the actual fix for this are Lifecycle Events where instead of navigating the <iframe> we blow away the application directly, which avoids trigger beforeunload events.

@brian-mann brian-mann removed this from the 2.0.0 milestone Feb 15, 2018
@brian-mann brian-mann mentioned this issue Feb 15, 2018
Closed
@jennifer-shehane
Copy link
Member

I thought this may have been fixed in 3.3.0 Cypress version, but it's not. Experiencing this same issue documented in electron:

@bondar-artem
Copy link

I have exactly the same issue with 3.3.0 with Electron only. Chrome works fine even the error message stays in the console

@hdavidzhu hdavidzhu mentioned this issue Jun 6, 2019
Closed
@jhays-nhaschools
Copy link

Related: #2118

@lots0logs lots0logs mentioned this issue Jul 27, 2019
Closed
@jennifer-shehane
Copy link
Member

Closing as duplicate of #2118

There are workarounds as well as a full explanation of this issue in this comment: #2118 (comment)

@jennifer-shehane jennifer-shehane added type: duplicate This issue or pull request already exists and removed stage: needs investigating Someone from Cypress needs to look at this type: bug labels Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: duplicate This issue or pull request already exists
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@brian-mann @jennifer-shehane @paulpruteanu @bondar-artem @jhays-nhaschools