Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122064
Message-ID: <f00e6612-5d2e-4c55-b7d0-55aa0cfec4db@gmail.com>
Date: Sat, 30 Dec 2023 00:37:31 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: "G. P. B." <george.banyard@gmail.com>
Cc: PHP internals <internals@lists.php.net>
References: <756bcf2b-f98d-4203-9004-1cbfd402337a@gmail.com>
 <CAFPFaMLaoZXnwzSYxrWiaWjhcwzLBZ4SHgswhatZ5f=Gm_jMew@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAFPFaMLaoZXnwzSYxrWiaWjhcwzLBZ4SHgswhatZ5f=Gm_jMew@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Pre-RFC: Fixing spec bugs in the DOM extension
From: dossche.niels@gmail.com (Niels Dossche)

Hi Gina

On 29/12/2023 15:40, G. P. B. wrote:
> Thank you for the work!
> 
> I agree that making them proper classes instead of aliases is the better proposition here.
> I'm not fully informed about the DOM spec, and I don't know if the current class/interface hierarchy is in the best shape, but maybe we should also consider having a look a this?
> 

Yeah, our current class hierarchy is wrong, but not "overly wrong". The incorrectness comes from the design of the pre-HTML5 era.

This is how it's supposed to be:
    CharacterData extends Node (Actually an interface, but PHP does not have interfaces with properties)
        Text extends CharacterData
            CDATASection extends Text
        ProcessingInstruction extends CharacterData
        Comment extends CharacterData

However in the current implementation, the ProcessingInstruction class extends Node instead of CharacterData.
Also CharacterData is a class instead of an interface in the current implementation.

So nothing too bad, but not correct either.
There's also some functionality that should be on the Element class instead of the Node class.

> About making those new classes finals, this would require reconsidering the class hierarchy anyway, as nearly everything inherits from DOMNode, and other classes (namely Comment/Text/CData nodes) extend other classes.
> However, I would not necessarily be against it, especially if we add the required interfaces, as the current mechanism of registering a custom class is not very powerful and rather cumbersome to use as the constructor is never called.

I'm already reconsidering the class hierarchy :-).
As for the constructor problem: I can fix that for the new classes, I can make sure the constructor is called which would already solve a pain point.

> As such, I'm not sure if I would support adding the current mechanism to customize the node classes returned by the extension. Indeed, the current mechanism doesn't play nicely at all with static analysis and this is something I stopped trying to integrate when writing my DocBook renderer project. [1]

I'm also not entirely sure, but in the JS world we do have custom elements that you can register and get an instance from back, so it has been done before at least.

> 
> Best regards,
> 
> Gina P. Banyard
> 
> [1] https://gitlab.com/Girgias/docbook-renderer <https://gitlab.com/Girgias/docbook-renderer>

Kind regards
Niels