dperini / nwsapi

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

case insensitive tag names doesn't seem to work after updating to 2.2.7 from 2.2.2 #100

Open Manvel opened 7 months ago

Manvel commented 7 months ago

Hey nwsapi team 👋

In our company we highly rely on JSDom(which seem to be using nwsapi for selectors resolution) and when doing the package resolution, we have realized that with update to nwsapi, the Uppercase tag name selector stopped working in our tests.

It seems like this issue has been reported previously and resolved, but apparently it's back(unless missing something).

dperini commented 6 months ago

@Manvel currently I fixed this for HTML docs only, previously we had it resolve correctly also for XML docs using:

        source = 'if(' + N + '(e.localName' +
          (Config.MIXEDCASE || hasMixedCaseTagNames(doc) ?
            '=="' + match[1].toLowerCase() + '"' :
            '=="' + match[1].toUpperCase() + '"') +
          ')){' + source + '}';

As you can see we have had a Config.MIXEDCASE configuration flag to force resolution in both HTML and XML documents. There was also an auto-detection method 'hasMixedCaseTagNames(doc)' used as alternate way to detect MixedCase. At that time it was decided to remove this functionality but we can easily reintroduce it.

Thank you for your help and contribution, much needed and appreciated.

Manvel commented 6 months ago

Thanks @dperini, think in that case current ticket and https://github.com/dperini/nwsapi/pull/101 can be closed?

Please let me know when the release is out, so I try following up with JSDom to update the dependencies.

Manvel commented 6 months ago

Hey @dperini 👋 Is there an ETA on the upcoming release by any chance? Thanks in advance.

dperini commented 6 months ago

@Manvel I am sorry I have been recovered to hospital since the beginning of this week. I couldn't do what I promise and this was the trend for all this last year. Though I don't want to give up and hope to be home next Monday.

Manvel commented 6 months ago

@Manvel I am sorry I have been recovered to hospital since the beginning of this week.

I couldn't do what I promise and this was the trend for all this last year.

Though I don't want to give up and hope to be home next Monday.

Ohh, sorry to hear that, no worries about delaying the release. If anything I can help lmk.

Manvel commented 2 months ago

hey @dperini just checking back on this, I assume there is still no progress here? I gave another try updating our tooling and hit same issue. So wanted to ping just in case.

dperini commented 2 months ago

@Manvel can you check and see if the latest change did fix your problem ? If it is not fixed submit a minimal example showing the problem. I will then be able to retake this issue by reintroducing older code. Thank you!