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

[John Hendrikx]

On Thu, 30 Mar 2023 22:34:44 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> The class `PseudoClassState` is private API, but was exposed erroneously in the CSS API. Instead, `Set<PseudoClass>` should have been used. This PR corrects this.
>
> modules/javafx.graphics/src/main/java/javafx/css/Match.java line 79:
> 
>> 77:      * @return the pseudo class state
>> 78:      */
>> 79:     public Set<PseudoClass> getPseudoClasses() {
> 
> Should this be an `ObservableSet`? Changing the type to the `Set` superclass will mean that applications would need to do an `instanceof` check to know whether it was observable or not?

I think that `Match` is supposed to be immutable, given the non-public constructor.  Match itself will never change the set (and nothing else will) so making it observable seems unnecessary.

However, this was not correctly spec'd, and you can change a `Match` now, and since it doesn't make a copy, you'd be changing the pseudo classes of whatever selector created it.  Note that `SimpleSelector` does make a copy, and goes to great pains to not expose anything mutable, but exposes it accidentally via `Match`.

In other words, `simpleSelector.createMatch().getPseudoClasses().clear()` would break the Selectors encapsulation.

I think it's best to close that loophole.  If you agree, I can document this method that it returns an immutable set, which is also what I assumed would be the case in my other PR where I made many of these immutable.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1070#discussion_r1153871673