Skip to content

Commit

Permalink
8266966: Wrong CSS properties are applied to other nodes after fix fo…
Browse files Browse the repository at this point in the history
…r JDK-8204568

Backport-of: 93de5840b19868fbe8850b846418c3f6f72df256
  • Loading branch information
arapte committed May 18, 2021
1 parent 37d25bc commit b2d930e
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 3 deletions.
Expand Up @@ -450,12 +450,15 @@ private StyleMap getStyleMap(Styleable styleable) {
private boolean forceSlowpath = false;
}

private boolean resetInProgress = false;

private void resetToInitialValues(final Styleable styleable) {

if (cacheContainer == null ||
cacheContainer.cssSetProperties == null ||
cacheContainer.cssSetProperties.isEmpty()) return;

resetInProgress = true;
// RT-31714 - make a copy of the entry set and clear the cssSetProperties immediately.
Set<Entry<CssMetaData, CalculatedValue>> entrySet = new HashSet<>(cacheContainer.cssSetProperties.entrySet());
cacheContainer.cssSetProperties.clear();
Expand All @@ -471,6 +474,7 @@ private void resetToInitialValues(final Styleable styleable) {
styleableProperty.applyStyle(calculatedValue.getOrigin(), calculatedValue.getValue());
}
}
resetInProgress = false;
}


Expand Down Expand Up @@ -586,9 +590,20 @@ private Set<PseudoClass>[] getTransitionStates(final Node node) {
// Any modifications to the method transitionToState() should be applied here if needed.
void recalculateRelativeSizeProperties(final Node node, Font fontForRelativeSizes) {

if (transitionStateInProgress) {
// If transitionToState() is being executed for the current control then all the css properties will get
// calculated there, and so we need to do anything here.
if (transitionStateInProgress || resetInProgress) {
// It is not required to recalculate the relative sized properties,
// 1. [transitionStateInProgress]: if transitionToState() is being executed for the current control then all
// the css properties will get calculated there, OR
// 2. [resetInProgress]: if resetToInitialValues() is being executed, which sets font to default font.
// The css style set by user if any is applied post this reset which calls
// recalculateRelativeSizeProperties() again.
// JDK-8266966: StyleManager.styleMapList stores the StyleMaps of nodes using an id as key.
// Each node stores this id in CssStyleHelper.CacheContainer.smapId
// CssStyleHelper.getStyleMap(node) gets a StyleMap from StyleManager.styleMapList by using the
// CssStyleHelper.CacheContainer.smapId as key.
// When resetToInitialValues() is in progress, the StyleManager.styleMapList gets updated, therefore
// calls to getStyleMap(node) should be avoided, as it may return an incorrect StyleMap for a given node.

return;
}
if (cacheContainer == null) {
Expand Down
111 changes: 111 additions & 0 deletions tests/system/src/test/java/test/javafx/scene/CssStyleHelperTest.java
@@ -0,0 +1,111 @@
/*
* Copyright (c) 2021, 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;

import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.control.Button;
import javafx.scene.control.CheckBox;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import junit.framework.Assert;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
import test.util.Util;
import static org.junit.Assert.assertTrue;

public class CssStyleHelperTest {

private static CountDownLatch startupLatch;
private static StackPane root;
private static Stage stage;
private static Label label1;
private static Button button;
private static CheckBox checkBox;
private static Label label2;

public static class TestApp extends Application {
@Override
public void start(Stage primaryStage) throws Exception {
stage = primaryStage;
label1 = new Label("Label1");
button = new Button("aButton");
checkBox = new CheckBox("aCheckBox");
label2 = new Label("Label2");

root = new StackPane();
root.getChildren().addAll(label1, button, checkBox, label2);
Scene scene = new Scene(root, 200, 200);
scene.getStylesheets().add(getClass().getResource("RootFont.css").toExternalForm());
primaryStage.setScene(scene);
primaryStage.setOnShown(l -> {
Platform.runLater(() -> startupLatch.countDown());
});
primaryStage.show();
}
}

@BeforeClass
public static void initFX() throws Exception {
startupLatch = new CountDownLatch(1);
new Thread(() -> Application.launch(TestApp.class, (String[]) null)).start();
assertTrue("Timeout waiting for FX runtime to start",
startupLatch.await(15, TimeUnit.SECONDS));
}

@Test
public void testCssIsCorrectlyAppliedToLabelOnStageHideAndShow() throws Exception {
// sanity
Assert.assertNull("Label1 should have no background", label1.getBackground());
Assert.assertNull("Label2 should have no background", label2.getBackground());

startupLatch = new CountDownLatch(1);
Util.runAndWait(() -> {
stage.hide();
stage.show();
});
assertTrue("Timeout waiting for Stage to show after hide",
startupLatch.await(15, TimeUnit.SECONDS));

Assert.assertNull("Label1 should have no background", label1.getBackground());
Assert.assertNull("Label2 should have no background", label2.getBackground());
}

@AfterClass
public static void teardownOnce() {
Platform.runLater(() -> {
stage.hide();
Platform.exit();
});
}
}
@@ -0,0 +1,3 @@
.root {
-fx-font-size: 50px;
}

1 comment on commit b2d930e

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.