Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8325704

Deprecate for removal SimpleSelector and CompoundSelector classes

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Approved
    • Icon: P4 P4
    • jfx23
    • javafx
    • None
    • source, behavioral
    • minimal
    • No risk for this change, classes were only deprecated, and a new method was introduced in a class that cannot be subclassed.
    • Java API
    • JDK

      Summary

      The SimpleSelector and CompoundSelector will be deprecated for removal to allow for future enhancements (see #JDK-8322964), and a new method will be introduced in their superclass Selector to cover a gap in the Selector API.

      Problem

      SimpleSelector and CompoundSelector have several hard to reach public methods intended for use by FX internals. Hard to reach because the only way to access them is to obtain a Selector (which is their superclass, and intended to be public) and then cast to a more specific type.

      This casting happens only in two places, in FX itself and in Scene Builder. Scene Builder is basically working around what could be considered a gap in the API of Selector (obtaining the style class names of a selector lacks API on Selector but it is available after casting to a specific type).

      The problem with this construction is this setup makes the SimpleSelector and CompoundSelector classes hard to evolve, because technically they're public API, but were never intended as such. This is corroborated by the fact they cannot be constructed directly (package private constructor), have public methods that are of questionable value for users, and are not directly accepted or returned as types anywhere in the rest of FX (only Selector is ever referred to).

      The motivation to improve this is a recent potential performance optimization described in https://bugs.openjdk.org/browse/JDK-8322964 -- this would require a new public method in SimpleSelector, which then unfortunately would become part of the exported API. To avoid this and to allow for the optimization in a future release, we are taking the steps to move these classes to internal packages. We're not doing this lightly, similar discussions were held in the past when other problems were addressed related to the selector classes.

      Solution

      The full solution will consist of several parts. The first part being to deprecate the SimpleSelector and CompoundSelector classes for removal, and to address the gap in the API of Selector by introducing a new method named getStyleClassNames to allow Scene Builder easy access to these without needing to cast to a specific type.

      The next step, to be performed in the next release, is to make the Selector superclass sealed permitting only the (moved to internal) types SimpleSelector and CompoundSelector. Although Selector is not final, it has a package private constructor allowing this to be done in a binary compatible fashion.

      As part of this process, some of the code in SimpleSelector and CompoundSelector that is accessing package private state in javafx.css will be moved directly to Selector.

      The full process for this next step was discussed here: https://github.com/openjdk/jfx/pull/1316#discussion_r1451738916

      Specification

      CompoundSelector changes:

      --- a/modules/javafx.graphics/src/main/java/javafx/css/CompoundSelector.java
      +++ b/modules/javafx.graphics/src/main/java/javafx/css/CompoundSelector.java
      @@ -60,7 +62,9 @@ import java.util.Set;
        * <code>Combinator.DESCENDANT</code>.
        *
        * @since 9
      + * @deprecated This class was exposed erroneously and will be removed in a future version
        */
      +@Deprecated(since = "23", forRemoval = true)
       final public class CompoundSelector extends Selector {
      
           private final List<SimpleSelector> selectors;
      @@ -97,6 +101,14 @@ final public class CompoundSelector extends Selector {
                       : Collections.EMPTY_LIST;
           }
      
      +    @Override
      +    public Set<String> getStyleClassNames() {
      +        return selectors.stream()
      +            .map(Selector::getStyleClassNames)
      +            .flatMap(Collection::stream)
      +            .collect(Collectors.toUnmodifiableSet());
      +    }
      +
           @Override public Match createMatch() {
               final PseudoClassState allPseudoClasses = new PseudoClassState();
               int idCount = 0;

      Selector changes:

      --- a/modules/javafx.graphics/src/main/java/javafx/css/Selector.java
      +++ b/modules/javafx.graphics/src/main/java/javafx/css/Selector.java
      @@ -91,6 +91,15 @@ abstract public class Selector {
               return ordinal;
           }
      
      +    /**
      +     * Gets the immutable set of style class names of this Selector.
      +     *
      +     * @return an immutable set with style class names, never {@code null},
      +     *     or contains {@code null}s, but can be empty
      +     * @since 23
      +     */
      +    public abstract Set<String> getStyleClassNames();
      +
           /**
            * Creates a {@code Match}.
            *

      SimpleSelector changes:

      --- a/modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java
      +++ b/modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java
      @@ -49,7 +50,9 @@ import static javafx.geometry.NodeOrientation.RIGHT_TO_LEFT;
        * A simple selector which behaves according to the CSS standard.
        *
        * @since 9
      + * @deprecated This class was exposed erroneously and will be removed in a future version
        */
      +@Deprecated(since = "23", forRemoval = true)
       final public class SimpleSelector extends Selector {
      
           /**
      @@ -192,6 +195,13 @@ final public class SimpleSelector extends Selector {
      
           }
      
      +    @Override
      +    public Set<String> getStyleClassNames() {
      +        return styleClassSet.stream()
      +            .map(StyleClass::getStyleClassName)
      +            .collect(Collectors.toUnmodifiableSet());
      +    }
      +
           @Override public Match createMatch() {
               final int idCount = (matchOnId) ? 1 : 0;
               int styleClassCount = styleClassSet.size();

            jhendrikx John Hendrikx
            kcr Kevin Rushforth
            Kevin Rushforth
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: