[2/2] msxml3: Support xmlns[:*] attribute nodes intelligently.
Ulrik Dickow
udickow at gmail.com
Tue Jun 12 03:55:02 CDT 2012
Patch 2 of 2 from http://bugs.winehq.org/show_bug.cgi?id=26226 .
--
Regards, Ulrik Dickow
-------------- next part --------------
From 8fc67ed3756eb938f070abd53ba880c35d9acc50 Mon Sep 17 00:00:00 2001
From: Ulrik Dickow <u.dickow at gmail.com>
Date: Mon, 11 Jun 2012 21:13:44 +0200
Subject: msxml3: Support xmlns[:*] attribute nodes intelligently.
This should solve bug #26226.
---
dlls/msxml3/domdoc.c | 34 +++++++-
dlls/msxml3/element.c | 110 +++++++++++++++++++++-----
dlls/msxml3/tests/domdoc.c | 190 +++++++++++++++++++++++++++++++++++++++++---
3 files changed, 301 insertions(+), 33 deletions(-)
diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c
index 5097ff6..44b23fd 100644
--- a/dlls/msxml3/domdoc.c
+++ b/dlls/msxml3/domdoc.c
@@ -1862,6 +1862,10 @@ static HRESULT WINAPI domdoc_createNode(
BSTR namespaceURI,
IXMLDOMNode** node )
{
+ static const WCHAR xmlnsW[] = {'x','m','l','n','s',0};
+ static const WCHAR xmlnscW[] = {'x','m','l','n','s',':'};
+ static const WCHAR w3xmlns[] = { 'h','t','t','p',':','/','/', 'w','w','w','.','w','3','.',
+ 'o','r','g','/','2','0','0','0','/','x','m','l','n','s','/',0 };
domdoc *This = impl_from_IXMLDOMDocument3( iface );
DOMNodeType node_type;
xmlNodePtr xmlnode;
@@ -1875,9 +1879,6 @@ static HRESULT WINAPI domdoc_createNode(
hr = get_node_type(Type, &node_type);
if(FAILED(hr)) return hr;
- if(namespaceURI && namespaceURI[0] && node_type != NODE_ELEMENT)
- FIXME("nodes with namespaces currently not supported.\n");
-
TRACE("node_type %d\n", node_type);
/* exit earlier for types that need name */
@@ -1893,6 +1894,33 @@ static HRESULT WINAPI domdoc_createNode(
break;
}
+ /* Emit appropriate fixme or warning in case of unsupported or invalid namespace.
+ * The W3C spec would like us to exit earlier for attempts to redefine the namespace for
+ * the reserved "xmlns" attribute or prefix (http://www.w3.org/TR/xml-names/#ns-decl),
+ * like http://gdome2.cs.unibo.it/gtk-doc/gdome2-gdomeelement.html#GDOME-EL-SETATTRIBUTENS,
+ * but since native msxml3 accepts anything here, it's safest to just continue with warning.
+ */
+ if(namespaceURI && namespaceURI[0] && node_type != NODE_ELEMENT)
+ {
+ if(node_type == NODE_ATTRIBUTE)
+ switch((!strcmpW(name, xmlnsW) ||
+ !strncmpW(name, xmlnscW, sizeof(xmlnscW)/sizeof(WCHAR))) +
+ !strcmpW(namespaceURI, w3xmlns))
+ {
+ case 0: /* Neither xmlns nor the reserved W3C xmlns URI */
+ FIXME("attribute nodes with namespaces not yet fully supported.\n");
+ break;
+ case 1: /* Either xmlns or the reserved URI, but not both -- illegal combination */
+ WARN("violates that prefix xmlns and nsURI %s belong together exclusively.\n",
+ debugstr_w(w3xmlns));
+ break;
+ case 2: /* xmlns has been paired with the appropriate URI, so be quiet */
+ break;
+ }
+ else
+ FIXME("type %d nodes with namespaces currently not supported.\n", node_type);
+ }
+
xml_name = xmlchar_from_wchar(name);
/* prevent empty href to be allocated */
href = namespaceURI ? xmlchar_from_wchar(namespaceURI) : NULL;
diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 26bfd0c..95e17be 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -1068,6 +1068,89 @@ static HRESULT WINAPI domelem_getAttribute(
return hr;
}
+static inline BOOL is_skippable_xlmns_attribute(
+ const xmlNodePtr element,
+ const xmlChar *name,
+ const xmlChar *value,
+ HRESULT *hr_ptr)
+{
+ /* Return TRUE if the parameter 'name' is a namespace definition attribute
+ * ("xmlns" or of the form "xmlns:PFIX") that denotes a prefix that is already
+ * associated with a non-empty namespace URI as seen from this element.
+ * If this non-empty URI equals the 'value' parameter,
+ * then silently set *hr_ptr to S_OK (no conflict, skip redundant attribute),
+ * else set it to a non-OK status and emit a warning/fixme depending on prefix or not:
+ * xmlns:PFIX : set it to E_INVALIDARG and emit a WARNing (skip conflicting attribute)
+ * xmlns : set it to E_NOTIMPL and emit a FIXME (native strangely allows this).
+ * When FALSE is returned (not a skippable xmlns attribute), hr_ptr is not accessed.
+ *
+ * Note: We would be more in line with the native msxml if we only detected a
+ * redundancy/conflict for prefixes existing on the element itself (in element->ns
+ * or in the linked list element->nsDef). But as long as our *_get_xml() doesn't
+ * eliminate redundant namespace definitions in children, this behaviour may be
+ * an advantage in some cases. As disadvantage is that we incorrectly regard the
+ * redefinition in element 'b' in '<a xmlns="x"><b xmlns="y"/></a>' as a conflict.
+ * Libxml2 currently doesn't have a function or option to only search for namespaces
+ * on the current node; but it would be easy to make a reduced version of xmlSearchNs
+ * (currently defined in http://git.gnome.org/browse/libxml2/tree/tree.c#n5871).
+ * However, most apps probably set the ns to y on node b creation, so no conflict here.
+ *
+ * See also http://bugs.winehq.org/show_bug.cgi?id=26226 .
+ */
+ static const xmlChar* xmlnsA = (const xmlChar*)"xmlns";
+ xmlChar *xml_prefix, *pfix = NULL; /* name="xmlns:a" => (xml_prefix="xmlns", pfix="a") */
+ xmlNsPtr ns;
+
+ if ((pfix = xmlSplitQName2(name, &xml_prefix)))
+ {
+ if (!xmlStrEqual(xml_prefix, xmlnsA))
+ {
+ xmlFree(xml_prefix);
+ xmlFree(pfix);
+ return FALSE; /* Attribute name had a prefix, but it was not "xmlns". */
+ }
+ xmlFree(xml_prefix); /* We only need pfix from now on (non-null here, free later). */
+ }
+ else
+ if (!xmlStrEqual(name, xmlnsA))
+ return FALSE; /* Attribute name had no prefix and didn't equal "xmlns". */
+
+ /* pfix is now the namespace prefix to search for. NULL means the default namespace. */
+ TRACE("pfix: %s\n", wine_dbgstr_a((const char *) pfix));
+
+ ns = xmlSearchNs(element->doc, element, pfix);
+ if (pfix) xmlFree(pfix); /* No longer needed, use ns->prefix instead. */
+
+ if (ns == NULL || ns->href == NULL)
+ return FALSE; /* Attribute name matched xmlns[:PFIX] but wasn't known, so don't skip. */
+
+ /* pfix already exists as a non-empty namespace, so attribute should be skipped (TRUE).
+ * Now decide whether it's a conflict/strangeness or just a redundancy.
+ */
+ if (xmlStrEqual(ns->href, value))
+ {
+ TRACE("skipping apparently redundant attribute\n");
+ *hr_ptr = S_OK; /* Important that the application believes it succeeded. */
+ }
+ else if (ns->prefix)
+ {
+ WARN("attribute %s attempts to change namespace URI for prefix %s from %s to %s\n",
+ debugstr_a((const char *) name),
+ debugstr_a((const char *) ns->prefix),
+ debugstr_a((const char *) ns->href),
+ debugstr_a((const char *) value));
+ *hr_ptr = E_INVALIDARG;
+ }
+ else
+ {
+ FIXME("not supported: adding xmlns=%s when element already has default namespace %s\n",
+ debugstr_a((const char *) value),
+ debugstr_a((const char *) ns->href));
+ *hr_ptr = E_NOTIMPL;
+ }
+ return TRUE;
+}
+
static HRESULT WINAPI domelem_setAttribute(
IXMLDOMElement *iface,
BSTR name, VARIANT value)
@@ -1209,11 +1292,10 @@ static HRESULT WINAPI domelem_setAttributeNode(
IXMLDOMAttribute** old)
{
domelem *This = impl_from_IXMLDOMElement( iface );
- static const WCHAR xmlnsW[] = {'x','m','l','n','s',0};
+ xmlNodePtr element = get_element(This);
xmlChar *name, *value;
BSTR nameW, prefix;
xmlnode *attr_node;
- xmlAttrPtr attr;
VARIANT valueW;
HRESULT hr;
@@ -1233,13 +1315,6 @@ static HRESULT WINAPI domelem_setAttributeNode(
hr = IXMLDOMAttribute_get_nodeName(attribute, &nameW);
if (hr != S_OK) return hr;
- /* adding xmlns attribute doesn't change a tree or existing namespace definition */
- if (!strcmpW(nameW, xmlnsW))
- {
- SysFreeString(nameW);
- return DISP_E_UNKNOWNNAME;
- }
-
hr = IXMLDOMAttribute_get_nodeValue(attribute, &valueW);
if (hr != S_OK)
{
@@ -1261,25 +1336,22 @@ static HRESULT WINAPI domelem_setAttributeNode(
name = xmlchar_from_wchar(nameW);
value = xmlchar_from_wchar(V_BSTR(&valueW));
+ hr = E_FAIL;
if (!name || !value)
+ hr = E_OUTOFMEMORY;
+ else if (!is_skippable_xlmns_attribute(element, name, value, &hr) &&
+ xmlSetNsProp(element, NULL, name, value))
{
- SysFreeString(nameW);
- VariantClear(&valueW);
- heap_free(name);
- heap_free(value);
- return E_OUTOFMEMORY;
- }
-
- attr = xmlSetNsProp(get_element(This), NULL, name, value);
- if (attr)
attr_node->parent = (IXMLDOMNode*)iface;
+ hr = S_OK;
+ }
SysFreeString(nameW);
VariantClear(&valueW);
heap_free(name);
heap_free(value);
- return attr ? S_OK : E_FAIL;
+ return hr;
}
static HRESULT WINAPI domelem_removeAttributeNode(
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index b942c9c..8f82497 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -7160,10 +7160,37 @@ static void test_testTransforms(void)
free_bstrs();
}
+/* Convenient helper function for creating an attribute node in a given namespace.
+ * Note: CHECK_HR() should not be here, but in caller, to give optimal line number info.
+ */
+static HRESULT create_attribute_ns(
+ IXMLDOMDocument *doc,
+ const char *attr_name,
+ const char *nsURI,
+ IXMLDOMAttribute **attr_ptr)
+{
+ HRESULT hr;
+ VARIANT var;
+ IXMLDOMNode *node;
+
+ V_VT(&var) = VT_I1;
+ V_I1(&var) = NODE_ATTRIBUTE;
+
+ hr = IXMLDOMDocument_createNode(doc, var, _bstr_(attr_name), _bstr_(nsURI), &node);
+ if (hr == S_OK)
+ {
+ IXMLDOMNode_QueryInterface(node, &IID_IXMLDOMAttribute, (void**) attr_ptr);
+ IXMLDOMNode_Release(node);
+ }
+ return hr;
+}
+
static void test_namespaces_change(int doc_version)
{
+ static const char *xmlnsURI = "http://www.w3.org/2000/xmlns/";
IXMLDOMDocument *doc;
- IXMLDOMElement *elem;
+ IXMLDOMElement *elem, *elem1, *elem2;
+ IXMLDOMAttribute *attr;
IXMLDOMNode *node;
VARIANT var;
@@ -7188,27 +7215,168 @@ static void test_namespaces_change(int doc_version)
hr = IXMLDOMDocument_get_documentElement(doc, &elem);
EXPECT_HR(hr, S_OK);
- /* try same prefix, different uri */
- V_VT(&var) = VT_BSTR;
- V_BSTR(&var) = _bstr_("ns/uri2");
+ IXMLDOMNode_Release(node);
- hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), var);
+ /* try same prefix, different uri, via setAttribute */
+ hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), _variantbstr_("ns/uri2"));
EXPECT_HR(hr, E_INVALIDARG);
- /* try same prefix and uri */
- V_VT(&var) = VT_BSTR;
- V_BSTR(&var) = _bstr_("ns/uri");
+ /* try same prefix and uri, via setAttribute */
+ hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), _variantbstr_("ns/uri"));
+ EXPECT_HR(hr, S_OK);
+
+ /* check xml so far */
+ hr = IXMLDOMElement_get_xml(elem, &str);
+ EXPECT_HR(hr, S_OK);
+ ok(!lstrcmpW(str, _bstr_("<ns:elem xmlns:ns=\"ns/uri\"/>")), "got element %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* try same prefix, different uri, via setAttributeNode */
+ hr = create_attribute_ns(doc, "xmlns:ns", xmlnsURI, &attr);
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMAttribute_put_nodeValue(attr, _variantbstr_("ns/uri2"));
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMElement_setAttributeNode(elem, attr, NULL);
+ EXPECT_HR(hr, E_INVALIDARG);
+
+ IXMLDOMAttribute_Release(attr);
+
+ /* try same prefix and uri, via setAttributeNode */
+ hr = create_attribute_ns(doc, "xmlns:ns", xmlnsURI, &attr);
+ EXPECT_HR(hr, S_OK);
- hr = IXMLDOMElement_setAttribute(elem, _bstr_("xmlns:ns"), var);
+ hr = IXMLDOMAttribute_put_nodeValue(attr, _variantbstr_("ns/uri"));
EXPECT_HR(hr, S_OK);
+ hr = IXMLDOMElement_setAttributeNode(elem, attr, NULL);
+ EXPECT_HR(hr, S_OK);
+
+ IXMLDOMAttribute_Release(attr);
+
+ /* check final xml (should be unchanged) */
hr = IXMLDOMElement_get_xml(elem, &str);
EXPECT_HR(hr, S_OK);
ok(!lstrcmpW(str, _bstr_("<ns:elem xmlns:ns=\"ns/uri\"/>")), "got element %s\n", wine_dbgstr_w(str));
SysFreeString(str);
IXMLDOMElement_Release(elem);
+
+ /* Now test NULL prefix (default namespace), via setAttributeNode, on two new elements. */
+
+ /* 1. test when element is created with empty namespace (parent element here). */
+ hr = IXMLDOMDocument_createElement(doc, _bstr_("elem1"), &elem1);
+ EXPECT_HR(hr, S_OK);
+
+ hr = create_attribute_ns(doc, "xmlns", xmlnsURI, &attr);
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMAttribute_put_nodeValue(attr, _variantbstr_("ns/uri1"));
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMElement_setAttributeNode(elem1, attr, NULL);
+ EXPECT_HR(hr, S_OK);
+
+ IXMLDOMAttribute_Release(attr);
+
+ /* check xml so far */
+ hr = IXMLDOMElement_get_xml(elem1, &str);
+ EXPECT_HR(hr, S_OK);
+ ok(!lstrcmpW(str, _bstr_("<elem1 xmlns=\"ns/uri1\"/>")), "got element %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* On native, get_namespaceURI still returns empty now. So do we. */
+ hr = IXMLDOMElement_get_namespaceURI(elem1, &str);
+ EXPECT_HR(hr, S_FALSE);
+ ok((str == NULL), "expected NULL, got %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* 2. test when element is created with non-empty namespace (child element here). */
+ V_VT(&var) = VT_I2;
+ V_I2(&var) = NODE_ELEMENT;
+
+ hr = IXMLDOMDocument_createNode(doc, var, _bstr_("elem2"), _bstr_("ns/uri1"), &node);
+ EXPECT_HR(hr, S_OK);
+
+ IXMLDOMNode_QueryInterface(node, &IID_IXMLDOMElement, (void**) &elem2);
+ IXMLDOMNode_Release(node);
+
+ hr = IXMLDOMElement_appendChild(elem1, (IXMLDOMNode*)elem2, NULL);
+ EXPECT_HR(hr, S_OK);
+
+ /* get_xml depends on context, for top node it omits child namespace attribute,
+ * but at child level it's still returned. Also true on native (before
+ * setAttributeNode on child) that the child namespace is suppressed by the parent
+ * setAttributeNode, even with NULL get_namespaceURI on parent.
+ */
+ hr = IXMLDOMElement_get_xml(elem1, &str);
+ EXPECT_HR(hr, S_OK);
+ todo_wine ok(!lstrcmpW(str, _bstr_("<elem1 xmlns=\"ns/uri1\"><elem2/></elem1>")),
+ "got element %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ hr = IXMLDOMElement_get_xml(elem2, &str);
+ EXPECT_HR(hr, S_OK);
+ ok(!lstrcmpW(str, _bstr_("<elem2 xmlns=\"ns/uri1\"/>")), "got element %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* 2a. try different uri, via setAttributeNode. Native allows this, resulting in
+ * different URI's from get_xml versus get_namespaceURI (like above, where get...URI empty).
+ * Don't know of any apps that depend on this strange behaviour, so maybe remain todo forever?
+ */
+ hr = create_attribute_ns(doc, "xmlns", xmlnsURI, &attr);
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMAttribute_put_nodeValue(attr, _variantbstr_("ns/uri2"));
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMElement_setAttributeNode(elem2, attr, NULL);
+ todo_wine EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMElement_get_xml(elem2, &str);
+ EXPECT_HR(hr, S_OK);
+ todo_wine ok(!lstrcmpW(str, _bstr_("<elem2 xmlns=\"ns/uri2\"/>")), "got element %s\n",
+ wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* On native, get_namespaceURI still returns ns/uri1 now. So do we. */
+ hr = IXMLDOMElement_get_namespaceURI(elem2, &str);
+ EXPECT_HR(hr, S_OK);
+ ok(!lstrcmpW(str, _bstr_("ns/uri1")), "expected ns/uri1, got %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ /* 2b. try same uri, via setAttributeNode (should be accepted without extra xml) */
+ hr = create_attribute_ns(doc, "xmlns", xmlnsURI, &attr);
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMAttribute_put_nodeValue(attr, _variantbstr_("ns/uri1"));
+ EXPECT_HR(hr, S_OK);
+
+ hr = IXMLDOMElement_setAttributeNode(elem2, attr, NULL);
+ EXPECT_HR(hr, S_OK);
+
+ /* After setAttributeNode, the child xmlns becomes visible from the parent on native too
+ * (or rather: it still shows the attribute xmlns instead of the element namespace,
+ * but here they happen to be identical).
+ */
+ hr = IXMLDOMElement_get_xml(elem1, &str);
+ EXPECT_HR(hr, S_OK);
+ todo_wine ok(!lstrcmpW(str, _bstr_("<elem1 xmlns=\"ns/uri1\"><elem2 xmlns=\"ns/uri1\"/></elem1>")),
+ "got element %s\n", wine_dbgstr_w(str));
+ /* Unfortunately wine currently inserts whitespace, while native doesn't.
+ * Shouldn't matter in the vast majority of apps, so relevant to test for the alternative too.
+ */
+ ok((!lstrcmpW(str, _bstr_("<elem1 xmlns=\"ns/uri1\"><elem2 xmlns=\"ns/uri1\"/></elem1>")) ||
+ !lstrcmpW(str, _bstr_("<elem1 xmlns=\"ns/uri1\">\r\n\t<elem2 xmlns=\"ns/uri1\"/>\r\n</elem1>"))),
+ "got element %s\n", wine_dbgstr_w(str));
+ SysFreeString(str);
+
+ IXMLDOMElement_Release(elem1);
+ IXMLDOMElement_Release(elem2);
IXMLDOMDocument_Release(doc);
+
+ free_bstrs();
}
static void test_namespaces(void)
@@ -7302,10 +7470,10 @@ static void test_namespaces(void)
IXMLDOMDocument_Release(doc);
+ free_bstrs();
+
test_namespaces_change(0);
test_namespaces_change(60);
-
- free_bstrs();
}
static void test_FormattingXML(void)
--
1.7.7.6
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 554 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20120612/9b4c81ba/attachment-0001.pgp>
More information about the wine-patches
mailing list