On Fri, 19 Jan 2024 10:21:05 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Moves `SimpleSelector` and `CompoundSelector` to internal packages.
>> 
>> This can be done with only a minor API break, as `SimpleSelector` and 
>> `CompoundSelector` were public before.  However, these classes could not be 
>> constructed by 3rd parties.  The only way to access them was by doing a cast 
>> (generally they're accessed via `Selector` not by their sub types).  The 
>> reason they were public at all was because the CSS engine needs to be able 
>> to access them from internal packages.
>> 
>> This change fixes a mistake (or possibly something that couldn't be modelled 
>> at the time) when the CSS API was first made public. The intention was 
>> always to have a `Selector` interface/abstract class, with private 
>> implementations (`SimpleSelector` and `CompoundSelector`).
>> 
>> This PR as said has a small API break.  The other changes are (AFAICS) 
>> source and binary compatible:
>> 
>> - Made `Selector` `sealed` only permitting `SimpleSelector` and 
>> `CompoundSelector` -- as `Selector` had a package private constructor, there 
>> are no concerns with pre-existing subclasses
>> - `Selector` has a few more methods that are now `protected` -- given that 
>> the class is now sealed, these modified methods are not accessible (they may 
>> still require rudimentary documentation I suppose)
>> - `Selector` now has a `public` default constructor -- as the class is 
>> sealed, it is inaccessible
>> - `SimpleSelector` and `CompoundSelector` have a few more `public` methods, 
>> but they're internal now, so it is irrelevant
>> - `createMatch` was implemented directly in `Selector` to avoid having to 
>> expose package private fields in `Match` for use by `CompoundSelector`
>> - No need anymore for the `SimpleSelectorShim`
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add since tags

In my opinion, Selector might be the most logical place to add the new methods.

This is a breaking change, so  I think we should talk about adding these 
methods at the same time as deprecating the classes, to give the tools 
developers time to switch to the new implementation and possibly re-packaging 
their app in a multi-release jar (it's always fun to make changes to the build 
scripts!).  

Will there be enough time for a complete review?

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

PR Comment: https://git.openjdk.org/jfx/pull/1333#issuecomment-1900711359

Reply via email to