On Fri, 19 May 2023 00:10:25 GMT, John Hendrikx <[email protected]> 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. > > John Hendrikx has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains three > new commits since the last revision: > > - Add tests from #1076 > - Merge remote-tracking branch 'upstream/master' into > feature/remove-private-api-in-css > - Fix possible regression 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. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1070#issuecomment-1555376730
