[PATCH v3] msxml3: Don't unlink the XML declaration when saving document.

[Dmitry Timoshkov]

v3:
Treat standalone == -1 and standalone == -2 same way when saving XML.

Signed-off-by: Dmitry Timoshkov 
---
 dlls/msxml3/domdoc.c       |  7 ++-----
 dlls/msxml3/tests/domdoc.c | 30 ++++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c
index 80c32e9ba99..19674d6713a 100644
--- a/dlls/msxml3/domdoc.c
+++ b/dlls/msxml3/domdoc.c
@@ -2534,7 +2534,6 @@ static HRESULT WINAPI domdoc_save(
 {
     domdoc *This = impl_from_IXMLDOMDocument3( iface );
     xmlSaveCtxtPtr ctx = NULL;
-    xmlNodePtr xmldecl;
     HRESULT ret = S_OK;

TRACE("(%p)->(%s)\n", This, debugstr_variant(&destination));
@@ -2567,7 +2566,7 @@ static HRESULT WINAPI domdoc_save(
             ret = IUnknown_QueryInterface(pUnk, &IID_IStream, (void**)&stream);
             if(ret == S_OK)
             {
-                int options = get_doc(This)->standalone == -1 ? XML_SAVE_NO_DECL : 0;
+                int options = get_doc(This)->standalone < 0 ? XML_SAVE_NO_DECL : 0;
                 ctx = xmlSaveToIO(domdoc_stream_save_writecallback,
                     domdoc_stream_save_closecallback, stream, NULL, options);

@@ -2583,7 +2582,7 @@ static HRESULT WINAPI domdoc_save(
     case VT_BSTR:
     case VT_BSTR | VT_BYREF:
         {
-            int options = get_doc(This)->standalone == -1 ? XML_SAVE_NO_DECL : 0;
+            int options = get_doc(This)->standalone < 0 ? XML_SAVE_NO_DECL : 0;

/* save with file path */
             HANDLE handle = CreateFileW( (V_VT(&destination) & VT_BYREF)? *V_BSTRREF(&destination) : V_BSTR(&destination),
@@ -2610,9 +2609,7 @@ static HRESULT WINAPI domdoc_save(
         return S_FALSE;
     }

-    xmldecl = xmldoc_unlink_xmldecl(get_doc(This));
     if (xmlSaveDoc(ctx, get_doc(This)) == -1) ret = S_FALSE;
-    xmldoc_link_xmldecl(get_doc(This), xmldecl);

/* will release resources through close callback */
     xmlSaveClose(ctx);
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index 5bc2fe1d5c3..85ea26699ee 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -8524,6 +8524,7 @@ static void test_createProcessingInstruction(void)
 {
     static const WCHAR xml1[] = L"<?xml version=\"1.0\"?>\r\n<test/>\r\n";
     static const char xml2[] = "<?xml version=\"1.0\" encoding=\"windows-1252\"?>\r\n<test/>\r\n";
+    static const char xml2_wine[] = "<?xml version=\"1.0\" encoding=\"windows-1252\"?>\n<test/>\n";
     IXMLDOMProcessingInstruction *pi;
     IXMLDOMDocument *doc;
     IXMLDOMNode *node;
@@ -8533,6 +8534,8 @@ static void test_createProcessingInstruction(void)
     VARIANT var;
     HRESULT hr;
     IStream *stream;
+    LARGE_INTEGER off;
+    VARIANT_BOOL b;
     HGLOBAL global;
     char *p;

@@ -8582,9 +8585,32 @@ todo_wine
     ok(hr == S_OK, "got 0x%08x\n", hr);
     p = GlobalLock(global);
     p[GlobalSize(global)] = 0;
-todo_wine
-    ok(!strcmp(p, xml2), "got %s\n", wine_dbgstr_a(p));
+    ok(!strcmp(p, xml2) || !strcmp(p, xml2_wine), "got %s\n", wine_dbgstr_a(p));
+    GlobalUnlock(global);
+
+    /* Verify the result after load+save */
+    off.QuadPart = 0;
+    hr = IStream_Seek(stream, off, STREAM_SEEK_SET, NULL);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+
+    hr = IXMLDOMDocument_load(doc, var, &b);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    ok(b == VARIANT_TRUE, "got %d\n", b);
+
+    off.QuadPart = 0;
+    hr = IStream_Seek(stream, off, STREAM_SEEK_SET, NULL);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+
+    hr = IXMLDOMDocument_save(doc, var);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+
+    hr = GetHGlobalFromStream(stream, &global);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    p = GlobalLock(global);
+    p[GlobalSize(global)] = 0;
+    ok(!strcmp(p, xml2) || !strcmp(p, xml2_wine), "got %s\n", wine_dbgstr_a(p));
     GlobalUnlock(global);
+
     IStream_Release(stream);

IXMLDOMDocument_Release(doc);

[Nikolay Sivov]

This appears to break stuff, example test that we should probably anyway:

---
{
    hr = IXMLDOMDocument_loadXML(doc, _bstr_("<?xml version=\"1.0\"
standalone=\"yes\"?>\r\n<test/>\r\n"), &b);
    ok(hr == S_OK, "got 0x%08x\n", hr);
    ok(b == VARIANT_TRUE, "got %d\n", b);

hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
    ok(hr == S_OK, "got 0x%08x\n", hr);

V_VT(&var) = VT_UNKNOWN;
    V_UNKNOWN(&var) = (IUnknown*)stream;
    hr = IXMLDOMDocument_save(doc, var);
    ok(hr == S_OK, "got 0x%08x\n", hr);

hr = GetHGlobalFromStream(stream, &global);
    ok(hr == S_OK, "got 0x%08x\n", hr);

p = GlobalLock(global);
    p[GlobalSize(global)] = 0;

ok(0, "output %s\n", wine_dbgstr_a(p));
}
---

Test failed: output "<?xml version=\"1.0\" standalone=\"yes\"?>\n<?xml
version=\"1.0\" standalone=\"yes\"?>\n<test/>\n"

This produces duplicate declaration after your change, but is fine with
current wine.

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> This appears to break stuff, example test that we should probably anyway:
>
> {
>     hr = IXMLDOMDocument_loadXML(doc, _bstr_("<?xml version=\"1.0\"
> standalone=\"yes\"?>\r\n<test/>\r\n"), &b);
>     ok(hr == S_OK, "got 0x%08x\n", hr);
>     ok(b == VARIANT_TRUE, "got %d\n", b);
>
> hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
>     ok(hr == S_OK, "got 0x%08x\n", hr);
>
> V_VT(&var) = VT_UNKNOWN;
>     V_UNKNOWN(&var) = (IUnknown*)stream;
>     hr = IXMLDOMDocument_save(doc, var);
>     ok(hr == S_OK, "got 0x%08x\n", hr);
>
> hr = GetHGlobalFromStream(stream, &global);
>     ok(hr == S_OK, "got 0x%08x\n", hr);
>
> p = GlobalLock(global);
>     p[GlobalSize(global)] = 0;
>
> ok(0, "output %s\n", wine_dbgstr_a(p));
> }
>
> Test failed: output "<?xml version=\"1.0\" standalone=\"yes\"?>\n<?xml
> version=\"1.0\" standalone=\"yes\"?>\n<test/>\n"
>
> This produces duplicate declaration after your change, but is fine with
> current wine.

Thanks. I've added your test case above and created a better patch that
passes this one as well.