Skip to content

soup: the attribute-operators empty-value rule has no test coverage #192

Closed

Description

@milo-oaklight

What

The empty-value rule for the attribute operators is documented but has no test coverage. _attr_operator_matches states it:

The empty-value cases follow the CSS spec: ~=, ^=, $= and *= never match an empty expected.

Removing the bool(expected) guard from any of the five operators leaves soup/test_soup_correctness.py green at 175 passed, checked one at a time. Nothing in the suite would notice.

Evidence

At 75d09b4 (the #182 merge; soup/soup.py is byte-identical to the reviewed 291042d), on

<p><b class="x" data-empty="">1</b><b class="x" data-v="a">2</b><b>3</b></p>
mutation suite selector before after soupsieve 2.10
^=: drop bool(expected) and 175 passed b[class^=""] [] ['1', '2'] []
$=: drop bool(expected) and 175 passed b[class$=""] [] ['1', '2'] []
*=: drop bool(expected) and 175 passed b[class*=""] [] ['1', '2'] []
~=: drop bool(expected) and 175 passed b[class~=""] [] [] []
|=: require a non-empty expected 175 passed b[data-empty|=""] ['1'] [] ['1']

The three substring operators are the ones that matter, and they fail in the direction #182 exists to prevent: with the guard gone, [class^=""] stops meaning "no match" and starts matching every element that carries the attribute. |= is load-bearing the other way, since its empty-value branch is what matches an empty attribute value, so that mutation under-matches instead. ~='s guard is inert either way, because "" in text.split() is already false.

Fix

Add the empty case to the operator matrix so it is compared against the reference like the rest of the matrix already is:

  • [attr~=""], [attr^=""], [attr$=""], [attr*=""] -> no match
  • [attr|=""] -> matches an empty attribute value

Against an attribute that is present and empty and one present and non-empty, since the present-and-empty case is what makes the rule visible at all.

Behaviour today is correct: all 15 attribute/operator combinations agree with soupsieve 2.10. This is a coverage gap rather than a defect, which is why #182 merged with it.

Related: #187 (legacy case-insensitive attributes), #191 (a quoted comma in a selector list).

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorCode refactoring, no behavior change

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions