8389092: PopupWindow.show() unconditionally overwrites user-set scene stylesheets with the owner's stylesheets

[Maran23]

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: #1394 (comment)

---

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.

jfx/modules/javafx.graphics/src/main/java/javafx/stage/PopupWindow.java

Lines 479 to 481 in 05a7b6d

	if (sceneValue.getCursor() == null) {
	sceneValue.setCursor(ownerScene.getCursor());
	}

Now, we will never overwrite anything that the developer set (before showing). And as we can see above, this was already done this way with the Cursor.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8389092: PopupWindow.show() unconditionally overwrites user-set scene stylesheets with the owner's stylesheets (Bug - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2236/head:pull/2236
$ git checkout pull/2236

Update a local copy of the PR:
$ git checkout pull/2236
$ git pull https://git.openjdk.org/jfx.git pull/2236/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2236

View PR using the GUI difftool:
$ git pr show -t 2236

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2236.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mhanl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 00: Full (c1ef0803)

[andy-goryachev-oracle]

not equivalent change: the old code set this.ownerWindow (to null in this case, see L467)

[andy-goryachev-oracle]

I am just a bit confused here. Referring to old version' line numbers, it looks like we should not have a null value returned from getRootWindow(owner): it's being dereferenced in L477 and also L485.

[andy-goryachev-oracle]

same thing - is it even possible for the rootWindow to be null?

[Maran23]

Yes, as you can see in the tests I wrote.