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

[Michael Strauß]

On Fri, 20 Oct 2023 00:26: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 two additional commits since the last revision:
> 
>  - 8185831: Minor change to remove extra line
>  - 8185831: Added testcase to verify when the selector ignores pseudo classes when the nodes have same style class

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

> 2033:     private boolean selectorMatches(Selector s) {
> 2034:         boolean matches = s != null && s.applies(this);
> 2035:         if(matches && !s.createMatch().getPseudoClasses().isEmpty()){

Please add a space after `if` and before `{`.

modules/javafx.graphics/src/test/java/test/javafx/scene/Node_lookup_Test.java line 65:

> 63:         f = new Group();
> 64:         f.getStyleClass().add("f");
> 65:         f.pseudoClassStateChanged(PseudoClass.getPseudoClass("testPseudo"),true);

Can you add a space after the comma in this line and other lines after that?

modules/javafx.graphics/src/test/java/test/javafx/scene/Node_lookup_Test.java line 202:

> 200:      */
> 201:     @Test
> 202:     public void lookupPseudoTest3() {

Consider naming the test methods according to what they test.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366383385
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366383814
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366385031