RFR: 8345188: Support tree-structural pseudo-classes [v6]

Michael Strauß mstrauss at openjdk.org
Wed Jan 8 17:35:03 UTC 2025


On Wed, 8 Jan 2025 12:02:45 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - Merge branch 'master' into feature/root-pseudo-class
>>  - whitespace error
>>  - add child-indexed pseudo-classes
>>  - add pseudo-class table to Parent
>>  - update cssref
>>  - Set :root pseudo-class on sub-scene root node
>>  - Set :root pseudo-class on root node
>
> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 381:
> 
>> 379:                     // the index of the first change to separate unchanged from changed elements.
>> 380:                     if (firstDirtyChildIndex < 0) {
>> 381:                         firstDirtyChildIndex = from;
> 
> Does this need some special casing in case the first element is modified indirectly?  Let's say I have two elements, I remove the last element.  Indirectly, the first element now gets the "only" and "last" pseudo classes; this may need to be communicated as a modification to the peer for example?

As long as the pseudoclasses are set correctly with `pseudoClassStateChanged`, the peers will be notified as necessary. I don't think there is any special-casing required here.

> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 470:
> 
>> 468:             // Add the "last-child" pseudo-class to the last child.
>> 469:             if (size() > 0) {
>> 470:                 getLast().pseudoClassStateChanged(LAST_CHILD_PSEUDO_CLASS, true);
> 
> These code paths seem to do double work.  Would it not be better to treat each flag separately, instead of having some paths manipulate multiple flags, that then may get set/reset again individually?  The first `if/else if` in this block will reset all flags, only to have some of them be set/reset again later.
> 
> This may also lead to listener confusion; let's say I have 2 children.  I remove the last child.  The "only child" block (`size() == 1`) triggers; it removes the `odd` pseudo class; a listener may trigger on this; however, just a bit later, this `odd` tag is then re-added (at least, I hope it is as I'm not entirely sure the `firstDirtyChildIndex` will be set low enough...)

I've refactored the implementation here a bit to prevent repeatedly clearing and setting a pseudo-class.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907549197
PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907551351


More information about the openjfx-dev mailing list