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

[Nikolay Sivov]

On 5/28/21 12:06 PM, Dmitry Timoshkov 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 have an application that reads the encoding from a PI node using
> IXMLDOMNamedNodeMap::getNamedItem(), and in addition it does some other
> things similar to what the provided tests do, that's why I implemented
> it this way, and I'd like to see full patchset to get in.
>
In that case sure, v3 is better (that's the one with 'properties' used
in PI node?). Please resend that one.