On 5/28/21 12:06 PM, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@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.