Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104691
MIME-Version: 1.0
References: <CAEPJdu=BuS8rf7_=GikUtCArdHSUD4AqY3+27tAJ1HPCP4Jm4A@mail.gmail.com>
In-Reply-To: <CAEPJdu=BuS8rf7_=GikUtCArdHSUD4AqY3+27tAJ1HPCP4Jm4A@mail.gmail.com>
Date: Wed, 13 Mar 2019 12:50:15 +0700
Message-ID: <CAEZPtU5r9oFDdDj_81+u+a2ZUQk3EjQ8FK1TvOnsODHGJUUVqw@mail.gmail.com>
To: Benjamin Eberlei <kontakt@beberlei.de>, Rob Richards <rrichards@ctindustries.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000fcaa760583f3613d"
Subject: Re: [PHP-DEV] On fixing DOMNameSpaceNode and DOM NS API Inconsistency Problems
From: pierre.php@gmail.com (Pierre Joye)

--000000000000fcaa760583f3613d
Content-Type: text/plain; charset="UTF-8"

Hi,

Only adding Rob just in case :)

best,

On Wed, Mar 13, 2019, 5:54 AM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> Hi everyone,
>
> While looking for things to work on in php-src my friend Thomas pointed me
> to a peculiar special case in ext/dom that leads to massive inconsistency
> problems in the API.
>
> There is an undocumented class DOMNameSpaceNode that gets returned from
> DOMElement::getAttributeNode(NS) if you select an attribute that represents
> a namespace (xmlns). This special case is intentionally handled in the
> code, contrary to Pythons DOM Extension which doesn't do this and contrary
> to the DOM Specification, which does not have a special DOMNameSpaceNode.
> Its all DOMAttr.
>
> Problematically DOMNameSpaceNode doesn't extend from DOMAttr, you cannot
> pass this class to DOMElement::removeAttributeNode(DOMAttr $attr):
>
>     Fatal error: Uncaught TypeError: Argument 1 passed to
> DOMElement::removeAttributeNode() must be an instance of DOMAttr, instance
> of DOMNameSpaceNode given
>
> Code example here: https://3v4l.org/jkC5s
>
> In addition the DOMNameSpaceNode renames all properties compared to
> DOMAttr, clearly violating the interface documented here
> http://php.net/manual/de/domelement.getattributenode.php
>
> Two potential fixes come to mind:
>
> 1. Have DOMNameSpaceNode extend DOMAttr - maybe this was the originally
> intended behavior, becuase the properties are all named differently?
> 2. Have DOMElement::removeAttributeNode accept also a DOMNameSpaceNode
> (3.) Remove the non standard DOMNameSpaceNode class and use a DOMAttr
> instead
>
> I think approach #1 is the right one from a BC perspective and also fixing
> the DOM API to be more consistent with the standard.
>
> But I am also not opposed to #3 in the longer term, looking at Github
> DOMNamespaceNode in the open source world is only used for one apc test, in
> PHP Compat and when dumping it in Symfony Var Dumper. I imagine its more
> deeply used in closed source code working deeply with XML.
>
> The second problem that this inconsistency creates is
> DOMElement::removeAttributeNS($namespaceUri, $localName). When you want to
> use it to remove a namespace attribute (xmlns). By default the xmlns
> namespace has the uri http://www.w3.org/2000/xmlns/. Hence removing
> xmlns:$name namespace would expected to be:
>
> DOMElement::removeAttributeNS("http://www.w3.org/2000/xmlns/", "foo") //
> removes xmlns:foo
>
> But thats not how it works, there is special code implemented that requires
> you to pick the URI from xmlns:foo value:
>
> <root xmlns:foo="urn:foo" />
>
> DOMElement::removeAttributeNS("urn:foo", "foo");
>
> But if your node has an attribute in this namespace with the same name, it
> gets removed as well:
>
> <root xmlns:foo="urn:foo" foo:foo="bar" />
>
> Would incorrectly become this, removing 2 attributes:
>
> <root />
>
> This is a bug that cannot be fixed in a BC way. I have no clue how to fix
> this completly broken behavior in a good way. I propose to break BC here
> and requiring "http://www.w3.org/2000/xmlns/" as namespaceURI from PHP 8.0
> to delete a namespace (xmlns) attribute.
>
> Should I put both issues into an RFC or are these rather bugs that can be
> fixed without an RFC?
>
> greetings
> Benjamin
>

--000000000000fcaa760583f3613d--