RFR: 8185831: Pseudo selectors do not appear to work in Node.lookupAll() [v11]

Kevin Rushforth kcr at openjdk.org
Fri Oct 27 16:47:46 UTC 2023


On Thu, 26 Oct 2023 23:27:08 GMT, Sai Pradeep Dandem <duke at openjdk.org> wrote:

>> **Issue:**
>> Using pseudo classes in programmatic query using Node.lookupAll() or Node.lookup() gives unexpected results.
>> 
>> **Cause:**
>> There is no check for checking the psuedo states matching in the applies() method of SimpleSelector.java. So checking for "applies()" alone is not sufficient in lookup() method.
>> 
>> **Fix:**
>> Included an extra check for the psuedo states to match.
>
> Sai Pradeep Dandem has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8185831: Changes to doc as per review comments

I left a couple comments on the API docs.

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1957:

> 1955:      * </p>
> 1956:      * <p>
> 1957:      *     If two nodes, lets say NodeA and NodeB have same style class "myStyle" and NodeA has

Since this is a separate paragraph, please add "For example, " here to make it clear that this is an example (and not normative specification). Maybe something like this:


     * For example, if two nodes, NodeA and NodeB, have the same ...

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1959:

> 1957:      *     If two nodes, lets say NodeA and NodeB have same style class "myStyle" and NodeA has
> 1958:      *     a pseudo state "myPseudo", then to find NodeA, the lookup method can be used as follows:
> 1959:      *     {@code scene.lookup(".myStyle:myPseudo");} or {@code scene.lookup(":myPseudo");}. If no pseudo class is specified

The sentence about what happens when no pseudo class is specified is true in general and not really part of this example. I recommend moving it before the two examples. And I think it could be worded more clearly.


    * If the lookup selector does not specify a pseudo class, the lookup will ignore pseudo class
    *  states; it will return the first matching node even if it contains pseudo classes.

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

PR Review: https://git.openjdk.org/jfx/pull/1245#pullrequestreview-1702154246
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1374804258
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1374811707


More information about the openjfx-dev mailing list