Hi rhabarberbarberah,

Welcome to ControlsFX and thank you for taking time to contribute to this project.

We do not recognise you as a contributor. Can you please sign ControlsFX Individual Contributor Agreement: https://cla.controlsfx.org ?

I did the CLA-Thing, let me know, if anything is missing

PopOver already adds event filter on owner window to hide without delay on WindowEvent.WINDOW_CLOSE_REQUEST and WindowEvent.WINDOW_HIDING events. Under what condition will these events fail and your additional listener help?

Hi @abhinayagarwal,
I just had a look at the section, you're right, that should -- in theory -- do the job and I missed that in the beginning, since I originally solved the problem from outside the class.
The filter doesn't seem to work as intended.
You should be able to reproduce the behaviour that was originally described in #448 by running the FX-Sample-Application.
Just close the Window while the PopOver is visible and you should see the described exception.

Taking a closer look it seems to make a difference, if you close the Window using "alt+F4", in that case the error doesn't seem to happen.

further investigating the ownerWindowClosing()-method doesn't seem to be executed, when you close the Window via the "x"-button

using the workaround I posted in the PR the error goes away.
Maybe it has something to do with how eventfilters are executed. Didn't dig deeper into that topic but I also noticed unexpected behaviour when playing around with eventfilters/-handlers & -listners in different contexts.
But I'm curious, does that give you a hint on the reason behind the error? maybe a more elegant solution?

Using just one Stage it might not seem that relevant to fix the error.
But in my usecase the whole application crashes when closing a subwindow.
--> that's why I was so keen to find a solution

Hi rhabarberbarberah,

Welcome to ControlsFX and thank you for taking time to contribute to this project.

We do not recognise you as a contributor. Can you please sign ControlsFX Individual Contributor Agreement: https://cla.controlsfx.org ?

I dug a little deeper, refactored the Case in parallel to my personal usecase.

1. I noticed, that the first try didn't help, when multiple Popovers are open at a time
   • solved by using eventfilter insted

also did some refactoring and cleanup

Important to note:
The existing implementation adds a WeakEventHandler. This approach doesn't seem to work for this case.
Also tested setups, where I tried out to replace EventHandlers/WeakEventHandlers in various places.
But they either don't solve the issue, or lead to additional exceptions.

do you have a SSCCE to test this PR?

Sure, not quite at hand, but I'm sure I can extract it off my project to reduce it to the essential part.
Will need some time. propably on weekend.

Hi @abhinayagarwal,
sorry, got distracted and temporary forgot, about this.
Here's a link to a short example, that should make it easy to test the issue.
Looking forward to your feedback.

https://github.com/rhabarberbarberah/PopOverBugFix

@abhinayagarwal Does the example help?
Are there any issues?

The example did help. I was trying to find the reason why it works with this fix and not with the old fix but haven't been able to. I am missing something.

Nevertheless, this approach works and I am OK to go forward with it. I have the following recommendations:

1. Change the InvalidateListener to ChangeListener. If the old value in change listener is not null, remove the event handlers from it
2. Remove the old fix from all the show() methods as they seem to be redundant:

ownerWindow.addEventFilter(WindowEvent.WINDOW_CLOSE_REQUEST,
                closePopOverOnOwnerWindowClose);
ownerWindow.addEventFilter(WindowEvent.WINDOW_HIDING,
                closePopOverOnOwnerWindowClose);

yes, you're right.
On the paper it looks like the old fix should do the job. I've also not been able to figure out, why it fails.

Hi rhabarberbarberah,

Welcome to ControlsFX and thank you for taking time to contribute to this project.

We do not recognise you as a contributor. Can you please sign ControlsFX Individual Contributor Agreement: https://cla.controlsfx.org ?

Hi,
I just pushed the recommended changes :)

Ah, this bot needs fixing :p

Thank you for the changes. I will test and merge them sometime soon.

Yea, was just gona ask about that.

still seems to run without exceptions.

While I removed the redundant lines from the show()-methods i noticed, that the ownerWindow field is reassigned in the process.
if the EventFilters would be set BEFORE reassignment, that could a reason, why they sometimes were not triggered. (although it doesn't seem to apply in our case)
I also noticed, that the ownerWindow field shadows a field in the PopupWindow-Class, which is referenced by the ownerWindowProperty(). ->suboptimal, because it makes it harder to understand what is referenced & overridden in different calls/assignments.
Might be connected to why the initial fix didn't always work.

You are absolutely right about ownerWindow field getting re-assigned in show(). However, addFilter statements are always present at the end i.e. after the re-assignment and call to super(), which means that both ownerWindow and getownerWindow() should (theoretically) have the same reference. I also tested this by checking the values in hide() method, leading to more frustration :)

Weak listeners are notorious in JavaFX and could also be a reason, so I got rid of them and used the listener closePopOverOnOwnerWindowCloseLambda in show methods but that didn't change anything :)

When parent is closed, hide() is called before WINDOW_CLOSE_REQUEST event is fired. This leads to removal of filters attached to the window events. This has been causing the whole ruckus. Thanks to @jperedadnr for digging it out.

The fix is quite simple, remove the following lines from hide():

if (ownerWindow != null){
        ownerWindow.removeEventFilter(WindowEvent.WINDOW_CLOSE_REQUEST,
                    closePopOverOnOwnerWindowClose);
        ownerWindow.removeEventFilter(WindowEvent.WINDOW_HIDING,
                closePopOverOnOwnerWindowClose);
}

I have made a commit with the fix. Please test it out and let me know if it works.

Hi @rhabarberbarberah ,

did you get a chance to test it?

Hey @abhinayagarwal,

just tested it with the sampler and my test application.
the issue seems to be fixed by this change.

greetings

Hi @rhabarberbarberah ,

Thank you for the test. I have simplified the fix and pushed it to this PR.

This was a long standing issue. Thank you for helping us to finally fix it 👍

Hi @abhinayagarwal,
this is great news.
looking forward to the next release, then I can remove my workaround from my project.