On Fri, 31 Jul 2026 20:29:49 GMT, Andy Goryachev <[email protected]> wrote:
>> `PopupWindow` will always overwrite its `Scene` (user agent) stylesheets >> when an owner was set and `show` is called. >> Code like this: >> >> >> final Popup popup = new Popup(); >> popup.getScene().setUserAgentStylesheet(popupUserAgentStylesheet); >> popup.getScene().getStylesheets().add(popupStylesheet); >> >> >> will do nothing, because your added stylesheets will be later overwritten >> when `show` is called. >> Andy and I were already wondering about this behavior two years ago: >> https://github.com/openjdk/jfx/pull/1394#discussion_r1617620949 >> >> --- >> >> I can't see any reason why we should do that. Instead, this PR will only add >> the stylesheets of the owner if they do not exist already. >> Additionally, we will not overwrite the user agent stylesheet if it was >> already set. >> >> Added tests for all combinations I can think of. This PR also fixes NPEs >> that can happen when the owner window has no 'root window'. >> >> There is already a test for the `Cursor` behavior, but there was none that >> verifies that the `Cursor` is not overwritten, so added one as well. >> >> https://github.com/openjdk/jfx/blob/05a7b6d0db5e799395da27ab43a93d4337001e11/modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java#L479-L481 >> >> Now, we will never overwrite anything that the developer set (before >> `show`ing). And as we can see above, this was already done this way with the >> `Cursor`. >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java line 462: > >> 460: private void showImpl(final Window owner) { >> 461: Window rootWindow = getRootWindow(owner); >> 462: if (rootWindow == null) { > > not equivalent change: the old code set this.ownerWindow (to null in this > case, see L467) You have a point but I don't see how this could be a problem. I can't imagine a usecase where you want to call `show` just to reset the owner > modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java line 521: > >> 519: Scene scene = getScene(); >> 520: if (scene.getUserAgentStylesheet() == null && >> ownerScene.getUserAgentStylesheet() != null) { >> 521: >> scene.setUserAgentStylesheet(ownerScene.getUserAgentStylesheet()); > > there might be another issue: > - owner A has the user agent stylesheet A.css > - a popup with no stylesheet is shown with the owner A. A.css is shown > - the popup gets hidden > - either owner A changes the stylesheet to B.css, or the popup is reusing > with a different owner > - the popup gets shown > > since `scene.getUserAgentStylesheet()` is already A.css, this code does not > set the new stylesheet, leaving the popup with a wrong style. > > I know reusing the popup (context menu, etc.) is a bad idea, but > surprisingly, I saw this happen many times in the past. AFAIK, there is no way that you can actually change the user agent stylesheet. It is read once and then cached. See e.g. https://github.com/openjdk/jfx/pull/525, where a user proposed to change that. So I think we are actually good here ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2236#discussion_r3741359184 PR Review Comment: https://git.openjdk.org/jfx/pull/2236#discussion_r3741357225
