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

Dmitry Timoshkov dmitry at baikal.ru
Fri May 28 04:06:52 CDT 2021


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.

-- 
Dmitry.



More information about the wine-devel mailing list