Skip to content

attr(), addClass(), hasAttr() and other attribute/class methods fatal on non-element nodes in the match set #98

Closed

Description

@gravitypdf-bot

Several attribute and class methods call element-only DOM methods (getAttribute(), setAttribute(), hasAttribute(), removeAttribute()) on every node in the match set without checking its type. When the set contains a text, comment, CDATA or processing-instruction node, as contents() returns, they fail with a fatal Error. tag() had the same bug (#93), but there it only produced a warning.

Reproduce

$q = qp('<?xml version="1.0"?><root>text<a class="c" x="1"/></root>', 'root')->contents();
$q->addClass('c');
// Error: Call to undefined method DOMText::hasAttribute()

Results on main (cd0de71, PHP 8.5) for two match sets. "Text first" is contents() of <root>text<a class="c" x="1"/></root>; "element first" is contents() of <root><a class="c" x="1"/>text</root>:

Call Where Text first Element first
attr('x') QueryMutators.php:989 Error: DOMText::getAttribute() '1'
attr('x', '1') QueryMutators.php:972 Error: DOMText::setAttribute() same Error
attr() QueryMutators.php:952 Warning: foreach() argument must be of type array|object, then [] works
addClass('c') QueryMutators.php:606 Error: DOMText::hasAttribute() same Error
removeClass('c') QueryMutators.php:660 Error: DOMText::hasAttribute() same Error
css('color', 'red') QueryMutators.php:1043 Error: DOMText::getAttribute() same Error
hasClass('c') QueryChecks.php:141 Error: DOMText::hasAttribute() true (it stops at the first hit)
hasAttr('x') QueryChecks.php:170 Error: DOMText::hasAttribute() same Error
removeAttr('x') QueryChecks.php:196 Error: DOMText::removeAttribute() same Error

The setters, hasAttr() and removeAttr() loop over every match, so one non-element node anywhere in the set breaks them. The getters read only the first match, so they break only when a non-element node is first.

Suggested fix

Fix this once at the boundary rather than with a guard in each method, as commit 745a4e2 did for the CSS engine. For example:

  • add a DOM helper that returns the first match that is a DOMElement, or null, plus a way to iterate only the element matches;
  • route tag() and the methods above through it.

That would make non-element nodes behave like jQuery's: setters skip them, and getters act as if they weren't in the set. For the getters, decide whether "first match" means the first node or the first element. #93 kept tag() reporting the first node and returning '' for a non-element. The getters here could do the same, returning null or false, or skip ahead to the first element; whichever is chosen, it should be applied consistently.

Found while reviewing #97.

Activity

  1. added 2 commits that reference this issue on Oct 25, 2026
    1b026ed
    6ae6acf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions