Re: mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation. (resend)

[Nikolay Sivov]

On 25.12.2015 10:30, Zhenbo Li wrote:
> +static void test_illegal_xml(IXMLDOMDocument *xmldom)
> +{
> - IXMLDOMNode *first, *last;
> - VARIANT variant;
> - HRESULT hres;
> - BSTR bstr;
> -
> - hres = IXMLDOMDocument_get_baseName(xmldom, NULL);
> - ok(hres == E_INVALIDARG, "Expect E_INVALIDARG, got %08x\n", hres);
> - hres = IXMLDOMDocument_get_baseName(xmldom, &bstr);
> - ok(hres == S_FALSE, "get_baseName failed: %08x\n", hres);
> - ok(bstr == NULL, "bstr(%p): %s\n", bstr, wine_dbgstr_w(bstr));
> - SysFreeString(bstr);
> -
> - hres = IXMLDOMDocument_get_dataType(xmldom, NULL);
> - ok(hres == E_INVALIDARG, "Expect E_INVALIDARG, got %08x\n", hres);
> - hres = IXMLDOMDocument_get_dataType(xmldom, &variant);
> - ok(hres == S_FALSE, "get_dataType failed: %08x\n", hres);
> - ok(V_VT(&variant) == VT_NULL, "got %s\n", debugstr_variant(&variant));
> - VariantClear(&variant);
> -
> - hres = IXMLDOMDocument_get_text(xmldom, &bstr);
> - ok(!strcmp_wa(bstr, ""), "text = %s\n", wine_dbgstr_w(bstr));
> - SysFreeString(bstr);
> -
> - hres = IXMLDOMDocument_get_firstChild(xmldom, NULL);
> - ok(hres == E_INVALIDARG, "Expect E_INVALIDARG, got %08x\n", hres);
> -
> - first = (void*)0xdeadbeef;
> - hres = IXMLDOMDocument_get_firstChild(xmldom, &first);
> - ok(hres == S_FALSE, "get_firstChild failed: %08x\n", hres);
> - ok(first == NULL, "first != NULL\n");
> -
> - last = (void*)0xdeadbeef;
> - hres = IXMLDOMDocument_get_lastChild(xmldom, &last);
> - ok(hres == S_FALSE, "get_lastChild failed: %08x\n", hres);
> - ok(last == NULL, "last != NULL\n");
>
> +}

It makes no sense to test every method on empty tree, test for text string.
> - hres = IXMLDOMDocument_QueryInterface(xmldoc, &IID_IObjectSafety, (void**)&safety);
> - assert(SUCCEEDED(hres));
> - hres = IObjectSafety_SetInterfaceSafetyOptions(safety, NULL,
> -
>    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER,
> -
>    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER);
> - assert(SUCCEEDED(hres));
> - IObjectSafety_Release(safety);

This looks weird to me, why the asserts? Did Jacek suggest that?

mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation. (resend)

[Zhenbo Li]

Hello Nikolay,

Thank you for viewing my patch on Christmas.

2015-12-26 6:56 GMT+08:00 Nikolay Sivov :
> On 25.12.2015 10:30, Zhenbo Li wrote:
> > +static void test_illegal_xml(IXMLDOMDocument *xmldom)
> > -
> > - last = (void*)0xdeadbeef;
> > - hres = IXMLDOMDocument_get_lastChild(xmldom, &last);
> > - ok(hres == S_FALSE, "get_lastChild failed: %08x\n", hres);
> > - ok(last == NULL, "last != NULL\n");
> >
> > +}
>
> It makes no sense to test every method on empty tree, test for text string.

I'm not sure that whether Windows will return an empty tree when
handling an illegal xml,
or being tolerant, return a non-empty tree.
> > - hres = IXMLDOMDocument_QueryInterface(xmldoc, &IID_IObjectSafety, (void**)&safety);
> > - assert(SUCCEEDED(hres));
> > - hres = IObjectSafety_SetInterfaceSafetyOptions(safety, NULL,
> > -
> >    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER,
> > -
> >    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER);
> > - assert(SUCCEEDED(hres));
> > - IObjectSafety_Release(safety);
>
> This looks weird to me, why the asserts? Did Jacek suggest that?

This is discussed before wine 1.8
http://wine.1045685.n5.nabble.com/Re-mshtml-Add-IHTMLXMLHttpRequest-response...

mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation. (resend)

[Nikolay Sivov]

On 26.12.2015 4:43, Zhenbo Li wrote:
> Hello Nikolay,
>
> Thank you for viewing my patch on Christmas.
>
> 2015-12-26 6:56 GMT+08:00 Nikolay Sivov :
> > On 25.12.2015 10:30, Zhenbo Li wrote:
> > > +static void test_illegal_xml(IXMLDOMDocument *xmldom)
> > > -
> > > - last = (void*)0xdeadbeef;
> > > - hres = IXMLDOMDocument_get_lastChild(xmldom, &last);
> > > - ok(hres == S_FALSE, "get_lastChild failed: %08x\n", hres);
> > > - ok(last == NULL, "last != NULL\n");
> > >
> > > +}
> >
> > It makes no sense to test every method on empty tree, test for text string.
>
> I'm not sure that whether Windows will return an empty tree when
> handling an illegal xml,
> or being tolerant, return a non-empty tree.

Still, it's enough to check children list length for example.
> > > - hres = IXMLDOMDocument_QueryInterface(xmldoc, &IID_IObjectSafety, (void**)&safety);
> > > - assert(SUCCEEDED(hres));
> > > - hres = IObjectSafety_SetInterfaceSafetyOptions(safety, NULL,
> > > -
> > >    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER,
> > > -
> > >    INTERFACESAFE_FOR_UNTRUSTED_CALLER | INTERFACESAFE_FOR_UNTRUSTED_DATA | INTERFACE_USES_SECURITY_MANAGER);
> > > - assert(SUCCEEDED(hres));
> > > - IObjectSafety_Release(safety);
> >
> > This looks weird to me, why the asserts? Did Jacek suggest that?
>
> This is discussed before wine 1.8
> http://wine.1045685.n5.nabble.com/Re-mshtml-Add-IHTMLXMLHttpRequest-response...

Ok, I'm pretty sure I didn't ask for it myself, but I'll leave it up to
Jacek.