dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
103 stars 35 forks source link

Rearrange useraction group to match `focus-within` before `focus` #74

Closed theKashey closed 7 months ago

theKashey commented 1 year ago

Currently a style with :focus-within ends as :focus + -within generating incorrect rule

The code before was producing for something like

.class1:focus-within .class2

something like

(function anonymous(s) {
"use strict";return function Resolver(c,f,x,r){var e,n,o;e=c;if((e.nodeName.toLowerCase()=="focus")){if((e===s.doc.activeElement&&s.doc.hasFocus()&&(e.type||e.href||typeof e.tabIndex=="number"))){if((/(^|\s)eGeHBG(\s|$)/.test(e.getAttribute("class")))){r=true;}}}return r;}
})

Where e.nodeName.toLowerCase()=="focus" is definitely not what expected

dospunk commented 1 year ago

I'd love to see this pulled soon! This issue has had me struggling for a few days

dperini commented 1 year ago

@theKashey I will first check if hooking the RegExp to start "^" and to the end "$" could solve the problem. In the case this doesn't work I will bend to the above alternative, thus rearranging the checking order.

theKashey commented 1 year ago

According to stack overflow ^ will enforce longest-match, so please try

theKashey commented 1 year ago

Solved by https://github.com/dperini/nwsapi/commit/8834c5e5b45d345154545e3b41937d35c83f9e74

theKashey commented 1 year ago

The original fix was reverted due to https://github.com/dperini/nwsapi/issues/80 From my own experiments, changing order is enough to greatly improve behaviour without capability of introducing loops.

dospunk commented 1 year ago

Any chance this can be reevaluated? Would love to see this bug fixed!

dperini commented 1 year ago

@dospunk Sure ... I am all for it. I thought this was fixed with the reordering of the user actions pseudo-class strings.. Can you please help and update me with a failing ":focus-within" test case that I can start with ?

dospunk commented 1 year ago

@dperini My bad, it does seem to be working now!

KatWorkGit commented 7 months ago

If there are comments about this working, should this issue be closed? I stumbled upon it from https://github.com/jsdom/jsdom/issues/3359

theKashey commented 7 months ago

The order is now correct. Not sure why this PR has't been marked as conflicted.