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

[Andy Goryachev]

On Wed, 18 Oct 2023 03:45:32 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: Fixed test failing issues and code review comments

Another question: this PR changes the behavior of Node.lookup() and Node.lookupAll() in respect to pseudo classes.
Now, wouldn't that pose a regression risk for applications?
Should we create a new method with the new semantics instead (+ boolean considerPseudoClasses or some such)?

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

> 1991:      */
> 1992:     List<Node> lookupAll(Selector selector, List<Node> results) {
> 1993:         if (selectorMatches(selector)) {

line 1990 might be incorrect: the return value **can** be null.
We probably should just correct the javadoc, and since it's a private API no CSR is needed.  All callers of this method do check for null.

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

> 106: 
> 107:     @Test
> 108:     public void lookupPsuedoTest(){

spelling: lookupPseudoTest
(also in the bug and PR title)

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

PR Comment: https://git.openjdk.org/jfx/pull/1245#issuecomment-1769133936
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1364317319
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1364312507