Re: mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation. (try 2)

[Jacek Caban]

Hi Zhenbo,

On 11/27/15 15:49, Zhenbo Li wrote:
> 2015-11-14 17:33 GMT+08:00 Zhenbo Li :
> > > Also, I found that there was a hidden bug:
> > > As we will test these urls for responseXML:
> > >     static const char xml_url[] = "http://test.winehq.org/tests/xmltest.xml";
> > >     static const char large_page_url[] =
> > > "http://test.winehq.org/tests/data.php";
> > >
> > > char large_page_url is not a valid xml file, but native mshtml.dll can
> > > handle it properly. It shows we can't just past the responseText to
> > > msxml3.dll. I'll check it more carefully it later.
>
> Hi, sorry for my late reply.
> I tested it on testbot, and found that Windows mshtml.dll would return
> an empty xml object

It looks better now.
> static BOOL doc_complete;
>  static IHTMLDocument2 *notif_doc;
> +static BOOL ILLEGAL_XML;

Using this global variable makes things harder to follow. Please add a
new argument to test_sync_xhr instead.
> @@ -334,8 +337,52 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseText(IHTMLXMLHttpRequest *i
>  static HRESULT WINAPI HTMLXMLHttpRequest_get_responseXML(IHTMLXMLHttpRequest *iface, IDispatch **p)
>  {
>      HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface);
> - FIXME("(%p)->(%p)\n", This, p);
> - return E_NOTIMPL;
> - IXMLDOMDocument *xmldoc = NULL;
> - BSTR str;
> - HRESULT hres;
> - VARIANT_BOOL vbool;
> - IObjectSafety *safety;
> -
> - TRACE("(%p)->(%p)\n", This, p);
> -
> - hres = CoCreateInstance(&CLSID_DOMDocument, NULL, CLSCTX_INPROC_SERVER, &IID_IXMLDOMDocument, (void**)&xmldoc);
> - if(FAILED(hres)) {
> -
>    ERR("CoCreateInstance failed: %08x\n", hres);
> -
>    return hres;
> - }
> -
> - hres = IHTMLXMLHttpRequest_get_responseText(iface, &str);
> - if(FAILED(hres)) {
> -
>    IXMLDOMDocument_Release(xmldoc);
> -
>    ERR("get_responseText failed: %08x\n", hres);
> -
>    return hres;
> - }
> -
> - hres = IXMLDOMDocument_loadXML(xmldoc, str, &vbool);
> - SysFreeString(str);
> - if(hres != S_OK || vbool != VARIANT_TRUE) {
> -
>    ERR("loadXML failed: %08x, returning an cmpty xmldoc\n", hres);
> - }

ERR is not appropriate here. Please use WARN instead.
> -
> - hres = IXMLDOMDocument_QueryInterface(xmldoc, &IID_IObjectSafety, (void**)&safety);
> - if(SUCCEEDED(hres)) {

Again, this error handling is more complicated than it needs to be.
Please use assert(hres == S_OK) instead.
> -
>    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);
> -
>    IObjectSafety_Release(safety);
> -
>    if(FAILED(hres)) {

Same here.

Thanks,
Jacek