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

Fix bug #54382 (getAttributeNodeNS doesn't get xmlns* attributes) #2337

Closed
wants to merge 2 commits into
base: PHP-7.0
from

Conversation

2 participants
@aboks
Contributor

aboks commented Jan 25, 2017

The fix is based on the same strategy for handling namespace
declarations as used by getAttributeNode. Note that this strategy makes
these methods not return a DOMAttr for xmlns* attributes, but an
instance of the (undocumented) class DOMNameSpaceNode. This is not
really ideal, but at least this fix makes the behavior of
getAttributeNode and getAttributeNodeNS consistent.

A follow-up action would be to investigate whether DOMNameSpaceNode can
be made into a subclass of DOMAttr (which may be hard due to the way
libxml treats namespace declarations) or document this deviating return
value for xmlns* attributes.

aboks added some commits Jan 25, 2017

Fix bug #54382 (getAttributeNodeNS doesn't get xmlns* attributes)
The fix is based on the same strategy for handling namespace
declarations as used by getAttributeNode. Note that this strategy makes
these methods not return a DOMAttr for xmlns* attributes, but an
instance of the (undocumented) class DOMNameSpaceNode. This is not
really ideal, but at least this fix makes the behavior of
getAttributeNode and getAttributeNodeNS consistent.

A follow-up action would be to investigate whether DOMNameSpaceNode can
be made into a subclass of DOMAttr (which may be hard due to the way
libxml treats namespace declarations) or document this deviating return
value for xmlns* attributes.

@krakjoe krakjoe added the Bugfix label Jan 25, 2017

@krakjoe

This comment has been minimized.

Member

krakjoe commented Jan 25, 2017

Merged 721a189

Thanks.

Please do a documentation patch on edit.php.net and ping me on this thread when done (with your username on edit).

@krakjoe krakjoe closed this Jan 25, 2017

php-pulls pushed a commit that referenced this pull request Jan 26, 2017

php-pulls pushed a commit that referenced this pull request Jan 26, 2017

Merge branch 'PHP-7.0' into PHP-7.1
* PHP-7.0:
  [ci skip] news entry for PR #2337

php-pulls pushed a commit that referenced this pull request Jan 26, 2017

Merge branch 'PHP-7.1'
* PHP-7.1:
  [ci skip] news entry for PR #2337
@aboks

This comment has been minimized.

Contributor

aboks commented Jan 30, 2017

@krakjoe I made a patch on edit.php.net for documenting this deviating return value. I've logged in on edit using my GitHub account (and got userID 77704).

@krakjoe

This comment has been minimized.

Member

krakjoe commented Jan 30, 2017

Merged, thanks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment