search

dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
107 stars 37 forks source link

Implement correct `:disabled` support for fieldsets and nested form controls #125

Closed camchenry closed 1 month ago

camchenry commented 2 months ago

With the current implementation of the :disabled selector, there are two main issues:

  1. We are not considering nested fieldsets, only checking the first ancestor fieldset. The HTML spec specifies that it should be any descendant:

    A form control is disabled if any of the following are true: [...] the element is a descendant of a fieldset element whose disabled attribute is specified, and is not a descendant of that fieldset element's first legend element child, if any.

  2. We are not correctly considering fieldsets that do not have a legend. Currently, the check for if the legend element contains the element doesn't handle if the first legend element doesn't exist in the fieldset. Specifically, (n=s.ancestor("fieldset",e))&&(n=s.first("legend",n))&&!n.contains(e)) returns false because n=s.first("legend",n) evaluates to null which is falsey, so the whole AND expression evaluates to false. Rather, if there is no legend element, we do not need to consider if the element is contained within the legend element and can solely focus on whether it is disabled or a descendant of a disabled fieldset.

Since the original test/w3c/html/semantics/selectors/pseudo-classes/disabled.html web-platform-test was added, it has been updated with additional test-cases. So I have added those here to ensure this works correctly.

dperini commented 2 months ago

@camchenry very well this is fantastic, this is real help for me. Thank you so much for the help catching the problem and code to fix it. It will be merged immediately after a small review once tested against wpt.

camchenry commented 2 months ago

@dperini Thanks, let me know if there is anything I can do to help with this PR. I did test it out in Chrome 127 on macOS and it looked okay, but I didn't try any other browsers so far.

dperini commented 1 month ago

@camchenry it is better if I apply your fix as is, without further delays since there have been enough testing. Optimizations can be applied later if they still make this change to pass wpt requirements.