JDK-8183100 : Styles not applied reliably after Java 8u92
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: 8u102,9,10
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: x86
  • Submitted: 2017-06-27
  • Updated: 2021-05-19
  • Resolved: 2017-12-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 8
10Fixed 8u172Fixed
Related Reports
Duplicate :  
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

Also happens in 8u111, 8u152 and 9+175. The bug did not affect Java 8u92.

ADDITIONAL OS VERSION INFORMATION :
Darwin Mac 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

However, we have also seen this bug on Windows and Linux.

EXTRA RELEVANT SYSTEM CONFIGURATION :
The example code uses Node#setStyle. In our actual application we use Application.setUserAgentStylesheet, but both ways of styling nodes are affected.

A DESCRIPTION OF THE PROBLEM :
Our JavaFX application uses a custom setup similar to a TabPane where we replace a container in our scene depending on the selected tab. The attached example resembles this structure.

However, if we add styled nodes to a container that we've just removed from the scene, then later add this container again, the styles of these added nodes are not applied consistently.

This is not a multithreading issue, and it always affects the same nodes. It's a pretty deterministic bug.

This regression might be related to the optimisation that addressed issue 8151756:

https://bugs.openjdk.java.net/browse/JDK-8151756

(Note the comment that has trouble with user-agent stylesheets in Java 8u>92)

REGRESSION.  Last worked in version 8u102

ADDITIONAL REGRESSION INFORMATION: 
8u92 works - we haven't tested versions between 92 (works) and 111 (broken)

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Run the attached program.

1. Click "Add and select new tab". This will open a tab with a yellow background.
2. Click "Add and select new tab" again. This will create a second tab, but will ALSO replace the contents of the first tab (now in the background) with a fresh yellow pane.
3. Click the first "Tab" in the tab bar to return to the first tab. The yellow background colour was not applied while the tab was in the background, and going back and forth between tabs does not fix this.
4. Every time a new tab is added, all existing tabs break.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Before opening any tabs, the bottom container should be _red_. After creating and switching between tabs, the bottom half of the screen should always be _yellow_.
ACTUAL -
The bottom half of the screen is _yellow_ when opening a fresh tab, but _fuchsia_ when returning to tab that was modified while in the background.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.util.*;
import javafx.application.*;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.layout.*;
import javafx.stage.Stage;

public class CSSBugDemo extends Application {
  static List<TabContents> TAB_CONTENTS = new ArrayList<>();

  static class TabContents extends StackPane {
    public TabContents() {
      // It is important that this node has some styling, either via setStyle() or getStyleClass().add().
      // It doesn't matter whether this is a valid style or just "-invalid-attribute: 0;"...
      setStyle("-fx-background-color: fuchsia");

      Platform.runLater(() -> {
        TAB_CONTENTS.add(this);
        TAB_CONTENTS.forEach(TabContents::fillWithFreshYellowPane);
      });
    }

    void fillWithFreshYellowPane() {
      Pane yellowPane = new Pane();
      yellowPane.setStyle("-fx-background-color: yellow");
      getChildren().setAll(yellowPane);
    }
  }
  
  @Override
  public void start(Stage primaryStage) {
    // Top half of the app: Horizontal navigation bar above the application.
    Button reapplyCSSButton = new Button("Reapply CSS");
    Button addTabButton = new Button("Add and select new tab");
    HBox tabBar = new HBox(reapplyCSSButton, addTabButton);

    // Bottom half of the app: The actual tab contents.
    StackPane container = new StackPane();
    // It is important that this node has some styling, either via setStyle() or getStyleClass().add().
    container.setStyle("-fx-background-color: red");
    VBox.setVgrow(container, Priority.ALWAYS);

    VBox root = new VBox(tabBar, container);

    // This needs to be removed for compilation with JDK 9.
    reapplyCSSButton.setOnAction(unused -> root.impl_reapplyCSS());

    ToggleGroup group = new ToggleGroup();

    addTabButton.setOnAction(unused -> {
      ToggleButton toggle = new ToggleButton("Tab");
      toggle.setToggleGroup(group);
      toggle.setSelected(true);
      tabBar.getChildren().add(toggle);

      TabContents contents = new TabContents();
      // Immediately select the new tab...
      container.getChildren().setAll(contents);
      // ...and select it again when clicking the toggle.
      toggle.setOnAction(actionEvent -> container.getChildren().setAll(contents));
    });

    primaryStage.setScene(new Scene(root, 500, 500));
    primaryStage.show();
  }

  public static void main(String[] args) { launch(args); }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Calling impl_reapplyCSS() fixes the bug by applying CSS again. This workaround is not available in JDK 9.


Comments
Looks good. Approved to backport to 8u-dev for 8u172.
15-12-2017

Here is the 8u-dev backport review request : http://cr.openjdk.java.net/~aghaisas/fx/8183100/8u/webrev.0/
14-12-2017

I have filed a bug to add regression test case : https://bugs.openjdk.java.net/browse/JDK-8193494
14-12-2017

This looks good to me. We need a unit test that will catch the visual regression. So as not to hold up getting this fix in for JDK 10, this can be done in a follow-up JBS issue, but should be done soon. Please file the following JBS bug for adding a regression test. +1
13-12-2017

Here is the bug that we shall use to address the CSS performance: https://bugs.openjdk.java.net/browse/JDK-8193445
13-12-2017

Fix for JDK 10 : http://cr.openjdk.java.net/~aghaisas/fx/8183100/webrev.1/ This patch reverts the patch of JDK-8151756. This fixes 3 regressions (JDK-8185709, JDK-8183100 and JDK-8168951) caused due to JDK-8151756. My patch webrev.0 does not retain the performance gained by the patch of JDK-8151756 As suggested by Kevin, I am choosing option 2 listed above. The performance issue needs to be revisited separately (I will open a bug and post the link below)
13-12-2017

In absence of a comprehensive test that shows both performance and correct styling - I have attempted to count Node.reapplyCss() invocations with the test programs attached to the regressions of JDK-8151756. Please refer to the attached image showing the count for different test programs. (reapplyCss_invocations.jpg) In this images, orange color indicates broken visual styling; green color indicates correct visual styling. It is obvious that the performance gained by JDK-8151756 will be undone by my fix. At this point of time, I do not have a better solution to fix all the regressions and preserve the performance gain.
12-12-2017

The test case I made for JDK-8151756 was pretty basic but shows what I observed in my full application. That's all I have that I can share. I could test with my the app, but I don't have any reason to think that the benefits from the fix for JDK-8151756 came from anything more than a reduction in the redundant calls to reapplyCSS. That would suggest that in terms of your #1 there is not likely to be any remaining performance win. Does Oracle have any performance test suite that would apply? How else do you guys measure for performance regressions? I wish I knew enough of the internals to offer more help, as I too don't want to have to undo the previous fix (#2). It isn't clear to me why redundant calls happen in the first place. I get that when the parent scene changes that CSS needs to be reapplied. It isn't clear why that doesn't just do a single pass over the added subtree.
01-12-2017

[~swpalmer] Do you have a more comprehensive test case for CSS performance that you could try (or that Ajit could try) to see if the proposed fix mentioned above still preserves part of the performance fix from JDK-8151756?
30-11-2017

I can confirm that the .00 patch fixes at least the test programs in this bug and in JDK-8168951. It looks like a safe enough fix for these problems. The two concerns I have are: A) Does this fix all of the issues, or are there still other (perhaps more subtle) problems still lurking? B) Does this retain enough of the original performance fix of JDK-8151756 to make that fix still effective? For at least the very simple test case attached to JDK-8151756 the answer is no. The number of calls to reapplyCSS for that simple test case are the same after applying your fix as it is with backing out the entire original fix (unless I did something wrong in my testing). I note that there are 4 options we might consider, and I would reject the last one: 1. Go with your proposed fix, and live with the performance drop in some cases. This seems a good option if we can convince ourselves that the fix is likely to address all outstanding regressions caused by the original fix (point A above), and as long as there is still some performance win from the remainder of the fix. 2. Backout the fix for JDK-8151756 entirely. This would be unfortunate, but is more certain to catch all potential problems than your proposed patch. If there is no measurable gain left with your patch then this might be an acceptable option. 3. Look for some condition / state change that can be used to qualify calling reapplyCSS for children when the parent's Scene changes. This seems unlikely for JDK 10 given the short timing, and would require very careful testing. 4. Do nothing for JDK 10 and defer this bug. I do not think this is a good option. This is too serious a bug to ignore it. Having a fast solution that is unreliable is not acceptable. I think for JDK 10 we are down to two choices: #1, go with your proposed fix, if we can come up with a test case that shows that it preserves part of the performance gain; or #2, backout the original fix for JDK-8151756.
30-11-2017

Raising the priority to P2, since the likelihood of applications hitting this bug is greater than we initially thought. There are already at least two other reported bugs that duplicate of this one.
30-11-2017

This patch fixes the issue : http://cr.openjdk.java.net/~aghaisas/fx/8183100/webrev.0/ What impact will it have on performance gained in JDK-8151756 need to be assessed.
28-11-2017

I think this is the same regression introduced with JDK-8151756
03-08-2017

See JDK-8185709 for another report that is likely due to this same regression.
02-08-2017

Issue reproducible in both windows and Linux. JDK results: ========== 8u92 : Pass 8u101-b13 : Pass 8u102-b14 : Fail <-- Regression introduced here 8u103: Fail 8u111 : Fail 8u121 : Fail 8u131 : Fail 9-ea+173 : Fail =========
28-06-2017