[PATCH v4 1/3] msxml3: Implement ::get_attributes() for "xml" processing instruction node.

[Nikolay Sivov]

On 5/28/21 11:37 AM, Dmitry Timoshkov wrote:
> Dmitry Timoshkov <dmitry at baikal.ru> wrote:
>
>> Nikolay Sivov <nsivov at codeweavers.com> wrote:
>>
>>>> @@ -45,6 +46,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(msxml);
>>>>  typedef struct _dom_pi
>>>>  {
>>>>      xmlnode node;
>>>> +    xmlNodePtr attributes;
>>>>      IXMLDOMProcessingInstruction IXMLDOMProcessingInstruction_iface;
>>>>      LONG ref;
>>>>  } dom_pi;
>>> Doesn't this create a situation when multiple msxml pi instances could
>>> be referencing same prolog node, but with different "attributes"
>>> contents? For tree consistency everything should be linked at libxml level.
>> That might be the case. Also, I've found that ::get_text() for such a node
>> returns data for main PI node instead of the special attributes one.
>>
>> At the mement I see only two possible solutions: keap using custom node
>> properties code like previous version of the patch does, or replace
>> node->type by XML_ELEMENT_NODE (like node.c,htmldoc_dumpcontent() does)
>> before calling libxml2's xmlSetNsProp/xmlFreeNode/xmlHasProp.
>>
>> I'm open for suggestions. Still, it seems to me that manual properties
>> management is cleaner than hacking node->type.
> I'm not sure how should I treat absence of your response. Also, I have no
> idea why you consider hacks in node.c,htmldoc_dumpcontent() with temporary
> patching node->type acceptable, but using legitimate node->properties
> storage with custom management like my previous implementation does as
> an abuse. I'm personally inclined to return back to v3 since it works
> in all cases.
>
> Also, I'd prefer to retain 3/3 as it is, and avoid introducing different
> XML declaration parsers.
It doesn't have to be different, you already have a function for that.
If the only thing you need is to save with correct encoding, you only
need to read that encoding. Node map for prolog node is nice to have,
but as you see yourself it goes against libxml2 tree management
concepts, when attaching properties to something that's not an element,
or creates inconsistency when you keep attributes at msxml instance, not
reflected in libxml2 tree.
>
> I'd like to see some more definitive answers/suggestions before I simply
> stop trying to upsteam these patches.
>
Saving with encoding read from prolog node was that suggestion.