diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/css/ControlCssTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/css/ControlCssTest.java new file mode 100644 index 00000000000..9f5fb8b42b6 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/css/ControlCssTest.java @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.javafx.scene.control.css; + +import com.sun.javafx.tk.Toolkit; +import javafx.css.CssParser; +import javafx.scene.Parent; +import javafx.scene.Scene; +import javafx.scene.control.Button; +import javafx.scene.control.Label; +import javafx.scene.control.Tab; +import javafx.scene.control.TabPane; +import javafx.scene.layout.Pane; +import javafx.scene.layout.StackPane; +import javafx.scene.layout.VBox; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import test.com.sun.javafx.scene.control.infrastructure.StageLoader; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Tests CSS together with Controls and their Skins. + */ +public class ControlCssTest { + + private StageLoader stageLoader; + + @AfterEach + void tearDown() { + if (stageLoader != null) { + stageLoader.dispose(); + } + } + + /** + * When we swap the root of a scene, it should still correctly resolve the CSS. + */ + @Test + void testLookupResolvesAfterSceneListenerRootSwap() { + var errors = CssParser.errorsProperty(); + errors.clear(); + + TabPane tabPane = new TabPane(); + Label label = new Label("Test"); + tabPane.getTabs().add(new Tab("TestTab", label)); + + AtomicBoolean swapped = new AtomicBoolean(false); + label.sceneProperty().addListener((_, _, newScene) -> { + if (newScene != null && !swapped.getAndSet(true)) { + Parent oldRoot = newScene.getRoot(); + StackPane newRoot = new StackPane(); + newScene.setRoot(newRoot); + newRoot.getChildren().setAll(oldRoot); + } + }); + + Button btn = new Button("Add Child"); + VBox root = new VBox(btn); + btn.setOnAction(_ -> root.getChildren().add(tabPane)); + + stageLoader = new StageLoader(new Scene(root)); + + btn.fire(); + Toolkit.getToolkit().firePulse(); + + assertEquals(0, errors.size(), errors::toString); + } + + /** + * When we swap a pane with a style class, the CSS for the children based of the Pane should correctly resolve. + */ + @Test + void testPaneClassLookupResolvesAfterSceneListenerPaneSwap() { + var errors = CssParser.errorsProperty(); + errors.clear(); + + String theme = toBase64(""" + .my-pane { + -color: #1B2631; + } + .my-pane .label { + -fx-text-fill: -color; + } + """); + + TabPane tabPane = new TabPane(); + Label label = new Label("Test"); + tabPane.getTabs().add(new Tab("TestTab", label)); + + StackPane myPane = new StackPane(); + myPane.getStyleClass().add("my-pane"); + + AtomicBoolean swapped = new AtomicBoolean(false); + label.sceneProperty().addListener((_, _, newScene) -> { + if (newScene != null && !swapped.getAndSet(true)) { + StackPane newPaneRoot = new StackPane(); + Pane paneRoot = (Pane) myPane.getParent(); + + paneRoot.getChildren().remove(myPane); + paneRoot.getChildren().add(newPaneRoot); + + myPane.getStyleClass().remove("my-pane"); + newPaneRoot.getStyleClass().add("my-pane"); + + newPaneRoot.getChildren().setAll(myPane); + } + }); + + Button btn = new Button("Add Child"); + VBox root = new VBox(btn, myPane); + btn.setOnAction(_ -> myPane.getChildren().add(tabPane)); + + Scene scene = new Scene(root); + scene.getStylesheets().add(theme); + stageLoader = new StageLoader(scene); + + btn.fire(); + Toolkit.getToolkit().firePulse(); + + assertEquals(0, errors.size(), errors::toString); + } + + private static String toBase64(String css) { + return "data:text/css;base64," + Base64.getEncoder().encodeToString(css.getBytes(StandardCharsets.UTF_8)); + } + +} diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java b/modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java index 888f86fd76a..33caed49a33 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java @@ -35,7 +35,6 @@ import java.util.Map.Entry; import java.util.Set; -import javafx.beans.value.WritableValue; import javafx.css.CssMetaData; import javafx.css.CssParser; import javafx.css.FontCssMetaData; @@ -75,6 +74,9 @@ /** * The StyleHelper is a helper class used for applying CSS information to Nodes. + * + * When created, a StyleHelper will always have a correct {@link #firstStyleableAncestor} set. + * It will be recreated when it changed and is therefore always correct and can be reliably used and trusted later on. */ final class CssStyleHelper { @@ -87,15 +89,34 @@ private CssStyleHelper() { * Creates a new StyleHelper. */ static CssStyleHelper createStyleHelper(final Node node) { + boolean userSetFont; + if (node.styleHelper == null) { + // Node styleHelper can not be reused later, because it does not exist. + // We can therefore safely set this true and ignore the property for the rest of this method. + userSetFont = true; + } else { + userSetFont = isUserSetFont(node); + } - // need to know how far we are to root in order to init arrays. - // TODO: should we hang onto depth to avoid this nonsense later? - // TODO: is there some other way of knowing how far from the root a node is? + Node styleableAncestor = null; Styleable parent = node; int depth = 0; - while(parent != null) { + while (parent != null) { depth++; parent = parent.getStyleableParent(); + + if (parent instanceof Node parentNode) { + if (styleableAncestor == null && isStyleableAncestor(parentNode)) { + styleableAncestor = parentNode; + } + if (!userSetFont) { + userSetFont = isUserSetFont(parentNode); + } + } + } + + if (node.styleHelper != null) { + setFirstStyleableAncestor(node.styleHelper, styleableAncestor); } // The List should only contain entries for those @@ -115,8 +136,7 @@ static CssStyleHelper createStyleHelper(final Node node) { // // reuse the existing styleHelper if possible. // - if ( canReuseStyleHelper(node, styleMap) ) { - + if (canReuseStyleHelper(node, styleMap, styleableAncestor)) { // // JDK-8123731 // @@ -127,7 +147,7 @@ static CssStyleHelper createStyleHelper(final Node node) { // trigger a REAPPLY. If the REAPPLY comes because of a change in font, then the fontSizeCache // needs to be invalidated (cleared) so that new values will be looked up for all transition states. // - if (node.styleHelper.cacheContainer != null && node.styleHelper.isUserSetFont(node)) { + if (userSetFont) { node.styleHelper.cacheContainer.fontSizeCache.clear(); } @@ -137,7 +157,6 @@ static CssStyleHelper createStyleHelper(final Node node) { updateParentTriggerStates(node, depth, triggerStates); return node.styleHelper; - } if (styleMap == null || styleMap.isEmpty()) { @@ -173,6 +192,7 @@ static CssStyleHelper createStyleHelper(final Node node) { } final CssStyleHelper helper = new CssStyleHelper(); + setFirstStyleableAncestor(helper, styleableAncestor); if (triggerStates[0] != null) { helper.triggerStates.addAll(triggerStates[0]); @@ -182,14 +202,12 @@ static CssStyleHelper createStyleHelper(final Node node) { helper.cacheContainer = new CacheContainer(node, styleMap, depth); - helper.firstStyleableAncestor = new WeakReference<>(findFirstStyleableAncestor(node)); - // If this node had a style helper, we need to reset all properties that will be unset with the // new style map to their initial values. Properties that remain set with the new style map carry // over to the new style helper. if (node.styleHelper != null) { Map remainingProperties = - node.styleHelper.resetToInitialValues(node, styleMap); + node.styleHelper.resetToInitialValues(node, styleMap); helper.cacheContainer.cssSetProperties.putAll(remainingProperties); } @@ -204,93 +222,77 @@ private static void updateParentTriggerStates(Styleable styleable, int depth, Ps for(int n=1; n 0) { - + if (triggerState != null && !triggerState.isEmpty()) { // Create a StyleHelper for the parent, if necessary. - // TODO : check why calling createStyleHelper(parentNode) does not work here? if (parentNode.styleHelper == null) { + // The createStyleHelper(..) is not used, because it can return null. + // But we do need a style helper to hold the triggerStates. parentNode.styleHelper = new CssStyleHelper(); - parentNode.styleHelper.firstStyleableAncestor = new WeakReference(findFirstStyleableAncestor(parentNode)) ; } parentNode.styleHelper.triggerStates.addAll(triggerState); - } - parent=parent.getStyleableParent(); + parent = parent.getStyleableParent(); } - } - // - // return true if the fontStyleableProperty's origin is USER - // - private boolean isUserSetFont(Styleable node) { - - if (node == null) return false; // should never happen, but just to be safe... - - CssMetaData fontCssMetaData = cacheContainer != null ? cacheContainer.fontProp : null; - if (fontCssMetaData != null) { - StyleableProperty fontStyleableProperty = fontCssMetaData != null ? fontCssMetaData.getStyleableProperty(node) : null; - if (fontStyleableProperty != null && fontStyleableProperty.getStyleOrigin() == StyleOrigin.USER) return true; - } - - Styleable styleableParent = firstStyleableAncestor.get(); - CssStyleHelper parentStyleHelper = getStyleHelper(firstStyleableAncestor.get()); - if (parentStyleHelper != null) { - return parentStyleHelper.isUserSetFont(styleableParent); - } else { + private static boolean isUserSetFont(Node node) { + if (node.styleHelper == null || node.styleHelper.cacheContainer == null) { return false; } + + StyleableProperty fontProperty = node.styleHelper.cacheContainer.getFontProperty(node); + return fontProperty != null && fontProperty.getStyleOrigin() == StyleOrigin.USER; } private static CssStyleHelper getStyleHelper(Node n) { return (n != null)? n.styleHelper : null; } - private static Node findFirstStyleableAncestor(Styleable st) { - Node ancestor = null; - Styleable parent = st.getStyleableParent(); - while (parent != null) { - if (parent instanceof Node) { - if (((Node) parent).styleHelper != null) { - ancestor = (Node) parent; - break; - } + private static Node getFirstStyleableAncestor(Styleable styleable) { + if (styleable instanceof Node node) { + WeakReference ancestorRef = node.styleHelper.firstStyleableAncestor; + if (ancestorRef != null) { + return ancestorRef.get(); } - parent = parent.getStyleableParent(); } - - return ancestor; + return null; } - // - // return the value of the property - // - private static boolean isTrue(WritableValue booleanProperty) { - return booleanProperty != null && booleanProperty.getValue(); + private static void setFirstStyleableAncestor(CssStyleHelper helper, Node ancestor) { + if (ancestor == null) { + helper.firstStyleableAncestor = null; + return; + } + helper.firstStyleableAncestor = new WeakReference<>(ancestor); } - // - // set the value of the property to true - // - private static void setTrue(WritableValue booleanProperty) { - if (booleanProperty != null) booleanProperty.setValue(true); + /** + * Whether {@code parentNode} can act as a styleable ancestor, i.e. whether it has a helper that + * can actually contribute styles. + */ + private static boolean isStyleableAncestor(Node parentNode) { + if (parentNode.cssHelperState == Node.CssHelperState.STALE) { + parentNode.cssHelperState = Node.CssHelperState.RESOLVED_EARLY; + parentNode.styleHelper = createStyleHelper(parentNode); + } + + return parentNode.styleHelper != null && parentNode.styleHelper.cacheContainer != null; } // // return true if the Node's current styleHelper can be reused. // - private static boolean canReuseStyleHelper(final Node node, final StyleMap styleMap) { + private static boolean canReuseStyleHelper(final Node node, final StyleMap styleMap, Node styleableAncestor) { // Obviously, we cannot reuse the node's style helper if it doesn't have one. if (node == null || node.styleHelper == null) { @@ -310,9 +312,6 @@ private static boolean canReuseStyleHelper(final Node node, final StyleMap style return false; } - //update ancestor since this node may have changed positions in the scene graph (JDK-8237469) - node.styleHelper.firstStyleableAncestor = new WeakReference<>(findFirstStyleableAncestor(node)); - // If the style maps are the same instance, we can re-use the current styleHelper if the cacheContainer is null. // Under this condition, there are no styles for this node _and_ no styles inherit. if (node.styleHelper.cacheContainer == null) { @@ -333,8 +332,7 @@ private static boolean canReuseStyleHelper(final Node node, final StyleMap style return true; } - CssStyleHelper parentHelper = getStyleHelper(node.styleHelper.firstStyleableAncestor.get()); - + CssStyleHelper parentHelper = getStyleHelper(styleableAncestor); if (parentHelper != null && parentHelper.cacheContainer != null) { int[] parentIds = parentHelper.cacheContainer.styleCacheKey.getStyleMapIds(); @@ -360,11 +358,13 @@ private static boolean canReuseStyleHelper(final Node node, final StyleMap style return false; } - private static final WeakReference EMPTY_NODE = new WeakReference<>(null); - /* This is the first Styleable parent (of Node this StyleHelper belongs to) - * having a valid StyleHelper */ - private WeakReference firstStyleableAncestor = EMPTY_NODE; + /** + * This is the first valid styleable ancestor of this helper. + * The first styleable ancestor is a styleable parent of the node (this helper belongs to) that has a + * styleHelper (with styles) and therefore might be important for styling this node. + */ + private WeakReference firstStyleableAncestor = null; private CacheContainer cacheContainer; @@ -395,7 +395,7 @@ private CacheContainer( // TODO: won't work for something like .menu-item:hover. Need to separate CssStyleHelper tree from scene-graph tree if ( parent instanceof Node) { Node parentNode = (Node)parent; - final CssStyleHelper helper = parentNode.styleHelper; + final CssStyleHelper helper = parentNode.styleHelper; if (helper != null && helper.cacheContainer != null) { smapIds[ctr++] = helper.cacheContainer.smapId; } @@ -434,7 +434,14 @@ private StyleMap getStyleMap(Styleable styleable) { } else { return StyleMap.EMPTY_MAP; } + } + + private StyleableProperty getFontProperty(Styleable node) { + if (fontProp == null) { + return null; + } + return fontProp.getStyleableProperty(node); } // This is the key we use to find the shared cache @@ -1302,21 +1309,21 @@ private CascadingStyle getInheritedStyle( final Styleable styleable, final String property) { - Styleable parent = ((Node)styleable).styleHelper.firstStyleableAncestor.get(); - CssStyleHelper parentStyleHelper = getStyleHelper((Node) parent); + Node ancestor = getFirstStyleableAncestor(styleable); + CssStyleHelper parentStyleHelper = getStyleHelper(ancestor); - if (parent != null && parentStyleHelper != null) { + if (ancestor != null && parentStyleHelper != null) { - StyleMap parentStyleMap = parentStyleHelper.getStyleMap(parent); - Set transitionStates = ((Node)parent).pseudoClassStates; - CascadingStyle cascadingStyle = parentStyleHelper.getStyle(parent, property, parentStyleMap, transitionStates); + StyleMap parentStyleMap = parentStyleHelper.getStyleMap(ancestor); + Set transitionStates = ancestor.pseudoClassStates; + CascadingStyle cascadingStyle = parentStyleHelper.getStyle(ancestor, property, parentStyleMap, transitionStates); if (cascadingStyle != null) { final ParsedValue cssValue = cascadingStyle.getParsedValue(); if ("inherit".equals(cssValue.getValue())) { - return getInheritedStyle(parent, property); + return getInheritedStyle(ancestor, property); } return cascadingStyle; } @@ -1347,20 +1354,17 @@ private CascadingStyle resolveRef(final Styleable styleable, final String proper } else { // TODO: This block was copied from inherit. Both should use same code somehow. - Styleable styleableParent = ((Node)styleable).styleHelper.firstStyleableAncestor.get(); - CssStyleHelper parentStyleHelper = getStyleHelper((Node) styleableParent); + Node ancestor = getFirstStyleableAncestor(styleable); + CssStyleHelper parentStyleHelper = getStyleHelper(ancestor); - if (styleableParent == null || parentStyleHelper == null) { + if (ancestor == null || parentStyleHelper == null) { return null; } - StyleMap parentStyleMap = parentStyleHelper.getStyleMap(styleableParent); - Set styleableParentPseudoClassStates = - styleableParent instanceof Node - ? ((Node)styleableParent).pseudoClassStates - : styleable.getPseudoClassStates(); + StyleMap parentStyleMap = parentStyleHelper.getStyleMap(ancestor); + Set styleableParentPseudoClassStates = ancestor.pseudoClassStates; - return parentStyleHelper.resolveRef(styleableParent, property, + return parentStyleHelper.resolveRef(ancestor, property, parentStyleMap, styleableParentPseudoClassStates); } } @@ -1873,12 +1877,13 @@ private CalculatedValue getCachedFont(final Styleable styleable) { // use the font property's value if it was set by the user and // there is not an inline or author style. - if (cacheContainer.fontProp != null) { - StyleableProperty styleableProp = cacheContainer.fontProp.getStyleableProperty(styleable); - StyleOrigin fpOrigin = styleableProp.getStyleOrigin(); - Font font = styleableProp.getValue(); - if (font == null) font = Font.getDefault(); + StyleableProperty fontProperty = cacheContainer.getFontProperty(styleable); + + if (fontProperty != null) { + StyleOrigin fpOrigin = fontProperty.getStyleOrigin(); if (fpOrigin == StyleOrigin.USER) { + Font font = fontProperty.getValue(); + if (font == null) font = Font.getDefault(); origin = fpOrigin; family = getFontFamily(font); size = font.getSize(); diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java index 76b8126fd59..ee79b180b6f 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/Node.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/Node.java @@ -1166,9 +1166,12 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) { focusSetDirty(oldScene); focusSetDirty(newScene); } + scenesChanged(newScene, newSubScene, oldScene, oldSubScene); - if (sceneChanged) reapplyCSS(); + if (sceneChanged) { + reapplyCSS(); + } if (sceneChanged && !isDirtyEmpty()) { //Note: no need to remove from scene's dirty list @@ -9819,9 +9822,22 @@ Map,List