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

Pass child window as an argument to the onUnload callback? #6

Open
anmcn opened this issue Aug 4, 2022 · 4 comments
Open

Pass child window as an argument to the onUnload callback? #6

anmcn opened this issue Aug 4, 2022 · 4 comments

Comments

@anmcn
Copy link

anmcn commented Aug 4, 2022

I have observed onUnload being called multiple times. I suspect, but have not been able to confirm, this happens when the pop-up window follows a redirect (attempts to debug with chrome devtools result in the behaviour changing).

In our application, we need to trigger certain actions when the popup closes, and this multiple triggering is causing those actions to be run prematurely. Ideally, onUnload would only be called when the pop-up actually closes, but this may be hard to achieve. If the child window was passed to the onLoad callback, one work-around would be to look at the child window.location.href, and only trigger those actions when the child window has reached it's final state.

@RyanNerd
Copy link
Owner

RyanNerd commented Aug 4, 2022

I think you've uncovered a bug. There's an event handler that fires when the popup window is being unloaded and onUnload(() gets called at this time. Then in the clean-up section there's code to implicitly call onUnload() again:

    // Clean up
    return () => {
      // Fire props.onUnload()
      if (onUnload) {
        onUnload();
      }

I'm working on a fix. Will release a new version as soon as I change and test. Probably in a couple of days.

Thanks for using react-new-improved-window

@anmcn
Copy link
Author

anmcn commented Aug 4, 2022

The strange thing is, I saw it called twice in Chrome, but only once in Firefox. That makes me wonder if the beforeUnload window event is being fired correctly in Firefox.

@RyanNerd
Copy link
Owner

RyanNerd commented Aug 4, 2022

I'll be sure to test on multiple browsers. It will be a shame if FF behavior doesn't fire the beforeUnload() event. I hate programming by exception (eg: if FF implicitly fire onUnload())

What if a new FF version fixes this then I need to add a version check as well. What a mess.

@RyanNerd
Copy link
Owner

@anmcn I tried a bunch of changes and each time the popup window closes the main window with it. 😠
I'm still working on fixing this but every change I've made makes it worse. I'll keep plugging away on it...
Just wanted you to know I was working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants