search

dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
105 stars 36 forks source link

Fix case insensetivity for attribute selectors #59

Closed ypapouski closed 1 year ago

ypapouski commented 2 years ago

The fix for the issue https://github.com/dperini/nwsapi/issues/60.

dperini commented 2 years ago

Ouch ... maybe this is still valid. I confused this to be part of the last regression which made selectors case-sensitive. But this is about attributes case-sensitivity. I will check this out asap.

dperini commented 2 years ago

@ypapouski thank you for your effort and contribution for this improvement. I find it strange the wpt do not have a test for detecting this inconsistency with attributes case sensitivity. I tested your changes with both my local test files and with those of jsdom + web_platform_tests and everything run smooth.

Now I find myself facing a minor problem that I wish to discuss with you as owner of this patch and contributor of the code change.

The minor problem is related to ES6 syntax you have used in this patch that chokes with the ancient ES3 syntax used previously, so I will be happy to have your suggestion here. I really like ES6 but for now I would like to maintain ES3 syntax for several reasons.

So to apply this patch do you prefer to change the syntax to ES3 and redo the pull request or should I do it myself ? Obviously, in both cases, I would keep the reference to you as the contributor of this patch. Appreciated your work and help refining this problem, thank you again !

ypapouski commented 2 years ago

@dperini I am glad to hear that my fix works correctly :)

About ES3 and ES6, well, I have no problems if you convert my changes made in ES6 into ES3.

Thanks for your feedback.

dperini commented 1 year ago

This was fixed in #60