On Thu, 27 Feb 2025 15:53:34 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> > For example, I recommend checking the following:
> > MenuBarSkin line 799
> > ComboBoxListViewSkin line 199
>
> I think it should be sufficient to mention the exception in the actual
> methods where the check is done, such as `hide()` and `show()`.
I'm not talking about docs here. What I meant that if the code in question is
ever executed on a background thread such that hide is called, this will now
cause an exception at runtime. I don't know whether it is possible, but it
isn't immediately obvious that this can't happen on a background thread -- at
least not for the two cases I mentioned. In fact, I'm pretty sure that at least
the second case (ComboBoxListViewSkin line 199) is possible.
lh.addInvalidationListener(comboBox.sceneProperty(), (o) -> {
if (((ObservableValue)o).getValue() == null) {
comboBox.hide();
}
});
I suspect that the following sequence of operations, all on a background
thread, will trigger the exception:
1. Create a ComboBox
2. Create a Skin
3. Add the combobox to a Scene (the scene must not be showing, of course)
A solution might be to check whether the combobox is showing before hiding it.
lh.addInvalidationListener(comboBox.sceneProperty(), (o) -> {
if (((ObservableValue)o).getValue() == null) {
if (comboBox.isShowing()) {
comboBox.hide();
}
}
});
And, if the code in MenuBarSkin is a problem, then something like this might be
in order:
if (menuButton.isShowing()) {
menuButton.hide();
}
Similarly, I think this listener in `MenuBarSkin::rebuildUI` (starting on line
913) might need a check before calling `menuButton.hide()`:
menuButton.menuListener = (observable, oldValue, newValue) -> {
if (menu.isShowing()) {
menuButton.show();
menuModeStart(container.getChildren().indexOf(menuButton));
} else {
menuButton.hide(); // <-- check if
(menuButton.isShowing()) before calling
}
};
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1717#issuecomment-2690955428
