-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert to v2.2.2 #104
Comments
As far as I tested, nwsapi does not handle the followings correctly.
|
@asamuzaK you are welcome to suggest changes/improvements that you see fits. |
@asamuzaK |
Could you please publish the pre-release version to npm? |
Do you mean publish a 2.2.10 release with the latest changes in github master or something else ? I was already planning a release for tomorrow afternoon and I would prefer that if you can wait just a few more hours. Validator problem is fixed and is the next fix I would commit on github later today, the problem was brought up by recent use of pseudo-classes names containing dashes (those should be escaped). A meaningful release is planned for tomorrow after some clean up and testing of the fixes for focus-xxx, dir(rtl/ltr), media states and remaining open issues. Many of the issue will be cleared by the validator fixes. I am sorry for the hassle this created in various projects, but I will do my best to have everything running smooth again with due additions and improvements with a new way/tool to test my work based on WPT testing suite that would give extra testing capabilities to have users help me in the maintenance of "nwsapi". New commits should start flowing in a few hours for you to review and suggest. Thank you in advance for your help and contributions. |
Great, I'll wait for it. As a side note, I recommend you to write unit tests for each commit. |
@asamuzaK |
Some feedback. First is the log of v2.2.2. Next is a comparison between v2.2.7 and v2.2.9. And the current master (commit 720c5e8) From above, I have to say that it has gotten worse and worse with each version since v2.2.2. I'm planning to cut a branch from v2.2.9 and help to fix below.
|
@asamuzaK |
Maybe it's because of my poor English, I'm frustrated with myself that I can't communicate well with you... The code in current nwsapi's master branch has too many regressions. For example, see Review source by asamuzaK · Pull Request #113 · dperini/nwsapi
|
@asamuzaK If you want I can talk in a conference call through Skype, I have it currently open and my alias name there is "nwbox_tech". It will be helpful to have a chat, writing is not as powerful in this case (due to a problem of mine). |
I told you that there are many regressions in the changes made after 2.2.9, so you should revert it.
No thank you. |
@asamuzaK |
Ref #115 |
- works correctly in browser but causes test failures - see dperini/nwsapi#104 (comment) - `:has()` support is apparently flaky, so we need to pin/revert the resolution
*:has(*:is(.classA, .classB)) + *:is(.classC, .classD) {
color: red;
} interestingly, the above works fine without the second edit: The following previous sibling selector also throws nwsapi errors: &:has(+ .classA) {
color: red;
} |
- works correctly in browser but causes test failures - see dperini/nwsapi#104 (comment) - `:has()` support is apparently flaky, so we need to pin/revert the resolution
- works correctly in browser but causes test failures - see dperini/nwsapi#104 (comment) - `:has()` support is apparently flaky, so we need to pin/revert the resolution
Most of the issue reports over the past year, e.g. #82, #83, #88, #89, #90, #95, #99, #100, #103 etc. indicate that the regular expressions used in v2.2.3 and later are incorrect.
I suggest reverting to v2.2.2 once and review the regexp.
The text was updated successfully, but these errors were encountered: