RFR: JDK-8304959: Public API in javafx.css.Match should not return private API class PseudoClassState [v9]

[Kevin Rushforth]

On Sat, 20 May 2023 00:57:08 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> I took a closer look at the compatibility issues Joe raised in the CSR, and I can confirm that yes, this change will break binary compatibility for any application that calls the existing method. It would also can break source compatibility for the subset of those applications that use the return value as an `ObservableSet` (which is the lowest public supertype of `PseudoClassState`), although this latter concern seems _very_ unlikely, and not all that impactful.
>> 
>> So it's the binary compatibility we need to consider. The question is: is the `Match::getPseudoClasses` method being called in a useful manner by applications today?
>> 
>> If not, then the change seems OK.
>> 
>> If it is, then we might need to consider adding a replacement method and deprecating the existing method for removal. The implementation of the replacement method would be exactly what your PR currently has for the existing method, and the existing method, would create a `PseudoClassState` object to hold a copy of the objects in the set.
>
>> So it's the binary compatibility we need to consider. The question is: is the `Match::getPseudoClasses` method being called in a useful manner by applications today?
>> 
>> If not, then the change seems OK.
>> 
>> If it is, then we might need to consider adding a replacement method and deprecating the existing method for removal. The implementation of the replacement method would be exactly what your PR currently has for the existing method, and the existing method, would create a `PseudoClassState` object to hold a copy of the objects in the set.
> 
> I've done some Googling with `"getPseudoClasses" import javafx`, and also looked at several code search sites, and if it's used, then I can't find any examples at all.  I also searched for `createMatch`, with similar results, no uses outside the JavaFX code base.
> 
> To call this method, one would to do something like this at a minimum:
> 
>      StyleSheet styleSheet = StyleSheet.loadBinary( <input stream> );
> 
>      for (Rule rule : styleSheet.getRules()) {
>           for (Selector selector : rule.getSelectors()) {
>                Match match = selector.createMatch();
>  
>                match.getPseudoClassStates();  // here...
>           }
>      }
> 
> I did find a few references to `loadBinary`, but nothing for `getRules` which would be needed next.
> 
> There's not a lot of reason to call `getPseudoClassStates` -- the method is mainly public so internal code outside its the public package can reach these to do the actual matching logic. This is why I initially tried to make `Selector`'s subclasses private, but couldn't.

@hjohn In case you missed seeing it, the CSR was approved, so Skara marked this PR as `ready`.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1070#issuecomment-1571025701