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

Electron browser hangs/errors on apps with window.onbeforeunload alerts #2118

Open
chheller opened this issue Jul 10, 2018 · 50 comments · Fixed by #5603
Open

Electron browser hangs/errors on apps with window.onbeforeunload alerts #2118

chheller opened this issue Jul 10, 2018 · 50 comments · Fixed by #5603
Labels
browser: electron E2E Issue related to end-to-end testing existing workaround prevent-stale mark an issue so it is ignored by stale[bot] stage: ready for work The issue is reproducible and in scope Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: bug

Comments

@chheller
Copy link

chheller commented Jul 10, 2018

Current behavior:

When running a test using the Electron browser on a webpage that contains a window.onbeforeunload script and trying to navigate away, Cypress will fail to load the next page. The test will eventually fail after a PageLoadTimeout exception. This occurs when using any kind of navigation such as cy.reload(), cy.visit(), or interacting with an element that contains an href linking to somewhere else.

Test run with Electron 59:

Desired behavior:

Test run with Chrome 67:

Steps to reproduce:

describe('Electron 59 alert prompt bug', function() {
  it('Should navigate away from the page', function() {
    cy.on('window:confirm', function() {
      return true;
    });
    cy.visit('http://localhost:8080/');
    cy.visit('http://localhost:8080/form');
  });
  it('Should reload the page', function() {
    cy.visit('http://localhost:8080/');
    cy.reload();
  });
});

The page under test needs to contain a window.onbeforeunload function which alters the returnValue. To reproduce, I use http-server to serve a barebones HTML file

<!DOCTYPE html>

<script>
  window.onbeforeunload = function (e) {
    e.returnValue = 'Dialog';
    return;
  }
</script>

Versions

Cypress: 3.0.2
MacOS: High Sierra 10.13.5
Browsers: Chrome 67, Electron 59

@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label Jul 10, 2018
@marmite22
Copy link

marmite22 commented Aug 21, 2018

I've just hit this exact same problem. For now I've wrapped my onBeforeUnload binding in a check for cypress which feels a bit dirty.

 if (!window.Cypress) {
    $window.onbeforeunload = function (e) { ... }
}

@bimimicah
Copy link

bimimicah commented Oct 4, 2018

I was able to work around the issue with the following code in the test:

cy.window().then((win) =>  {
	win.onbeforeunload = null;
})
cy.visit(<the next page goes here>)

@jennifer-shehane jennifer-shehane changed the title Electron 59 does not handle window navigation alerts Electron browser hangs/errors on apps with window.onbeforeunload alerts Nov 10, 2018
@jennifer-shehane jennifer-shehane added stage: ready for work The issue is reproducible and in scope and removed stage: needs investigating Someone from Cypress needs to look at this labels Nov 10, 2018
@jennifer-shehane
Copy link
Member

Reproducible example: cypress-io/cypress-test-tiny#27

@brandonb927
Copy link

Just chiming in here to say we have an issue within our tests because Cypress doesn't properly handle the window.onbeforeunload event in Electron w/ Chromium 59, but works perfectly fine using Chrome 70 running the test runner. In our Jenkins env using docker, it also experiences the same issue with the cypress/base:10 image.

My guess is that the version of Chromium used by Electron (59) that ships with Cypress currently (3.1.0) has some major bugs and issues. I see #2559 is acknowledged, any ETA when that might get rolled out? I imagine this would fix a swath of issues being worked around due to an old version of Chromium.

Ya'll doin' great work though, I really appreciate Cypress :)

Cypress: 3.1.0
macOS: 10.14.1
Browsers: Electron 59, Chrome 70

@Obi-Dann
Copy link

Obi-Dann commented Dec 7, 2018

Started encountering this problem in our CI build. For now going to workaround by monkey patching window.addEventListener:

Cypress.on('window:load', function(window) {
    const original = window.addEventListener;
    window.addEventListener = function() {
        if (arguments && arguments[0] === 'beforeunload') {
            return;
        }
        return original.apply(this, arguments);
    };
});

@Fonger
Copy link

Fonger commented Mar 16, 2019

If you're using window.onbeforeunload=function() {...} instead of window.addEventListener('beforeunload', function() {...}, you can use this workaround:

Cypress.on('window:load', function(window) {
  Object.defineProperty(window, 'onbeforeunload', {
    get: function() {},
    set: function() {}
  });
});

@fritz-c
Copy link

fritz-c commented May 23, 2019

Regarding dannsam's approach, you may want to change 'window:load' to 'window:before:load' if you are adding the beforeunload listener in some code executed on load.

@szymach
Copy link

szymach commented Jun 20, 2019

Thank you @DanNSam and @fritz-c , your workaround did it for me. I have encountered this issue using the autosave plugin for CKEditor 5, since it tries to make sure the last call was completed or something.

@bahmutov
Copy link
Contributor

bahmutov commented Jul 6, 2019

I have pushed another example https://github.com/cypress-io/cypress-test-tiny/tree/test-unload that shows the problem in Electron 61 (Chrome passes, despite showing an error). The website in question is http://webdriverjsdemo.github.io/leave/

and has the following script

window.onbeforeunload = function(){
  return 'Are you sure you want to leave?';
};

The error displayed in the browser console:

Screen Shot 2019-07-06 at 6 05 20 PM

This error has docs in https://www.chromestatus.com/feature/5082396709879808 and probably is not disabled in Electron

Funnily, you cannot close the Electron browser in GUI mode - it just keeps showing the error

Screen Shot 2019-07-06 at 6 05 38 PM

Running cypress run --headed shows the error that happens on CI - the test keeps running

Screen Shot 2019-07-06 at 6 08 12 PM

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Nov 4, 2019

There is also a situation where, where after visiting the site above, this will exhibit as a Page Load timeout.

Failing test code below:

describe('page', () => {
  it('works', () => {
    cy.visit('http://webdriverjsdemo.github.io/leave/')
    cy.contains('Go Home')
      .click()
  })

  it('works 2', () => {
    cy.visit('http://webdriverjsdemo.github.io/leave/')
      .contains('Go Home')
      .click()
  })
})

Screen Shot 2019-11-04 at 5 03 12 PM

Screen Shot 2019-11-04 at 5 00 17 PM

Electron will not display this error in the Console when this is happening also (if you have not manually interacted with the page)

[Intervention] 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

Workaround

There are 2 possible workarounds.

  1. Run your tests in Chrome using the --chrome flag during cypress run or choosing Chrome during cypress open.

OR

  1. Add the code below to your support/index.js file. This will prevent the prompts from occurring - and not hang Electron.
Cypress.on('window:before:load', function (win) {
  const original = win.EventTarget.prototype.addEventListener

  win.EventTarget.prototype.addEventListener = function () {
    if (arguments && arguments[0] === 'beforeunload') {
      return
    }
    return original.apply(this, arguments)
  }

  Object.defineProperty(win, 'onbeforeunload', {
    get: function () { },
    set: function () { }
  })
})

zifgu added a commit to tapestry-tool/tapestry-wp that referenced this issue Jul 14, 2022
- Implements a workaround described here: cypress-io/cypress#2118
@k4mr4n
Copy link

k4mr4n commented Jan 18, 2023

This worked for me

Cypress.on('window:before:unload', e => {
    e.stopImmediatePropagation()
})

@Joelasaur
Copy link

This issue is keeping me from using cy.origin(), since when I click the href to navigate to an external site, I can see the browser is loading the page, but cypress is still failing because of the page load timeout error. Unable to post a reproducer since it's proprietary code but here's an excerpt of what I'm doing:

    cy.get('[data-cy="my-external-site"]').click();
    cy.origin('https://my-external-site.com', () => {
      cy.url().should('include', 'my-external-site');
    });

According to the docs, the above code should work, but this page load timeout error is keeping me from using it.

@nagash77 nagash77 added the prevent-stale mark an issue so it is ignored by stale[bot] label Apr 3, 2023
@hemaybedead
Copy link

Putting the code from workaround number 2 into my beforeEach() block solved this for me.

@nagash77 nagash77 added E2E Issue related to end-to-end testing Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. labels May 18, 2023
@alecmestroni
Copy link

alecmestroni commented May 29, 2023

Putting the code from workaround number 2 into my beforeEach() block solved this for me.

Whats the workaround number 2? @hemaybedead

@alecmestroni
Copy link

The only workaround that worked for me is to extend the 'pageLoadTimeout' to 600000 in cypress.config.js

kamilmielnik added a commit to metabase/metabase that referenced this issue Oct 17, 2023
* Our app registers beforeunload event listener e.g. when editing a native SQL question.
* Cypress does not automatically close the browser prompt and does not allow manually
* interacting with it (unlike with window.confirm). The test will hang forever with
* the prompt displayed and will eventually time out. We need to work around this by
* monkey-patching window.addEventListener to ignore beforeunload event handlers.
*
* @see cypress-io/cypress#2118
kamilmielnik added a commit to metabase/metabase that referenced this issue Oct 17, 2023
)

* Update isNavigationAllowed logic to prevent new model creation

* Add simple model creation test

* Remove identifier

* Update comment

* Add a test case for saving a new model

* Avoid using fetchMock directly in QueryBuilder unit tests

* Remove setupCollectionsEndpointsByIds in favor of setupCollectionByIdEndpoint

* Add native question test base

* Update isNavigationAllowed tests

* Make isNavigationAllowed tests pass

* Update shouldShowUnsavedChangesWarningForSqlQuery logic to account for new questions

* Add a test case to run new native question

* Update mock to include individual collection GET endpoint

* Add DataSourceSelectors to MockNativeQueryEditor

* Update test beforeunload event to reflect new behavior

* Use setupCollectionByIdEndpoint instead of modifying setupCollectionsEndpoints

* Update failing test

* Use serializeCardForUrl

* Add a test for native query editing

* Do not trigger warning when leaving empty question

* Add beforeunload tests for creating new questions

* Remove redundant div

* Make assertions consistent

* Use single resetAllMocks()

* Fix post-rebase conflict

* Work around Cypress not handling beforeunload-related prompts

* Our app registers beforeunload event listener e.g. when editing a native SQL question.
* Cypress does not automatically close the browser prompt and does not allow manually
* interacting with it (unlike with window.confirm). The test will hang forever with
* the prompt displayed and will eventually time out. We need to work around this by
* monkey-patching window.addEventListener to ignore beforeunload event handlers.
*
* @see cypress-io/cypress#2118
@adnanerlansyah403
Copy link

Any updates on this issue guys ?, I have got the same issue too and I've already made the thread for this and provide a reproducable code. here's the link, I hope you'll can fix this as fast because this making me frustrating. I can't run the test on chrome's and edge browser and only working on firefox

#28449

@jennifer-shehane
Copy link
Member

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

@kbaala88
Copy link

kbaala88 commented Dec 13, 2023 via email

@adnanerlansyah403
Copy link

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

Oke, Thank you so much for your assitance. Please notice me or let me know if it's already fixed 🙏

@adnanerlansyah403
Copy link

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

Hey @jennifer-shehane sorry, how about now for the issue? Is there any update on this? How to progress in solving the problem?

@adnanerlansyah403
Copy link

adnanerlansyah403 commented Jan 22, 2024

Please keep it up for this issue, there are bunches of people occurring this.

@valscion
Copy link

valscion commented Jan 22, 2024

Posting repeatedly on the issue thread will not help you, @adnanerlansyah403. It will only work to annoy people like me who are subscribed to this thread to see if it gets resolved at some point.

Cypress team will prioritize the issues as they see fit. You can add a 👍 reaction to the original issue message to help Cypress team prioritize this issue and see how many people it affects.

If anything, adding more "please fix this" comments serves to make Cypress team less interested in fixing this issue.

Note

I am not associated with Cypress in any way — so these opinions are my own.

@jennifer-shehane
Copy link
Member

@valscion Commenting certainly doesn't make Cypress less interested in fixing an issue, but for longstanding issues, we do prioritize them based on impact and effort to fix and this is certainly a known issue in our backlog to be prioritized against other work.

There's this other issue with onbeforeunload alerts that's popped up and made this prevelant within Chrome browsers which has put this somewhat on our radar. #28553 But I'm not sure they can both be addressed together or not.

@adnanerlansyah403
Copy link

adnanerlansyah403 commented Jan 23, 2024

@valscion Commenting certainly doesn't make Cypress less interested in fixing an issue, but for longstanding issues, we do prioritize them based on impact and effort to fix and this is certainly a known issue in our backlog to be prioritized against other work.

There's this other issue with onbeforeunload alerts that's popped up and made this prevelant within Chrome browsers which has put this somewhat on our radar. #28553 But I'm not sure they can both be addressed together or not.

I'm glad this issue is already on the backlog, I hope this can be fixed as soon as possible. And if it's already fixed on the next update of cypress, please notice it too in the description. I appreciate so much for developers cypress for what they're doing to fix the issue like this.

@posti85
Copy link

posti85 commented Oct 7, 2024

I was facing the same issue. But I think the cause is different from the rest. My app is configured to be PWA, and the service worker was causing issues in the navigations (the cy.visit() call always hang).

More info about the problem and one of its workarounds here: https://www.youtube.com/watch?v=hmbx_f5Nlfs, people are talking about the same issue in this thread: #27501.

The workaround that worked for my was:

cy.intercept('**/registerSW.js', '');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser: electron E2E Issue related to end-to-end testing existing workaround prevent-stale mark an issue so it is ignored by stale[bot] stage: ready for work The issue is reproducible and in scope Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.