Re: mshtml.dll:Add implementation of HTMLDocument_(Get|Set)Title

[Jacek Caban]

Ivan Sinitsin wrote:
> Changelog:
>     mshtml.dll:Add implementation of HTMLDocument_(Get|Set)Title

First of all it would be nice if you could add a test case for these.
> static HRESULT WINAPI HTMLDocument_put_title(IHTMLDocument2 *iface, BSTR v)
> {
>     HTMLDocument *This = HTMLDOC_THIS(iface);
> - FIXME("(%p)->(%s)\n", This, debugstr_w(v));
> - return E_NOTIMPL;
> - nsIDOMDocument *nsdoc;
> - nsIDOMHTMLDocument *nshtmldoc;
> - nsAString nsstr;
> - nsresult nsres;
> -
> - TRACE("(%p)->(%s)\n", This, debugstr_w(v));
> -
> - if(!This->nscontainer)
> -
>    return E_POINTER;
> -
> - nsres = nsIWebNavigation_GetDocument(This->nscontainer->navigation, &nsdoc);
> - if(NS_FAILED(nsres)) {
> -
>    ERR("GetDocument failed: %08x\n", nsres);
> -
>    return nsres;
> - }
> -
> - if(NS_FAILED(nsres) || !nsdoc).
> -
>    return E_FAIL;
> -
> - nsIDOMDocument_QueryInterface(nsdoc, &IID_nsIDOMHTMLDocument, (void**)&nshtmldoc);
> - nsIDOMDocument_Release(nsdoc);
> -
> - nsAString_Init(&nsstr, NULL);
> - nsAString_SetData(&nsstr, v);

Use second argument of nsAString_Init instead of nsAString_SetData.
> -
> - nsres = nsIDOMHTMLDocument_SetTitle(nshtmldoc, &nsstr);
> - nsIDOMHTMLDocument_Release(nshtmldoc);
> -

You leak nsstr here.
> - return nsres;

Don't mix nsresult with HRESULT. You should return S_OK here.
> }
>
> static HRESULT WINAPI HTMLDocument_get_title(IHTMLDocument2 *iface, BSTR *p)
> {
>     HTMLDocument *This = HTMLDOC_THIS(iface);
> - FIXME("(%p)->(%p)\n", This, p);
> - return E_NOTIMPL;
> - nsIDOMDocument *nsdoc;
> - nsIDOMHTMLDocument *nshtmldoc;
> - nsAString nsstr;
> - const PRUnichar *ret;
> - nsresult nsres;
> -
> - TRACE("(%p)->(%p)\n", This, p);
> -
> - if (!p).
> -
>    return E_POINTER;
> -
> - if(!This->nscontainer)
> -
>    return E_POINTER;
> -
> - nsres = nsIWebNavigation_GetDocument(This->nscontainer->navigation, &nsdoc);
> - if(NS_FAILED(nsres)) {
> -
>    ERR("GetDocument failed: %08x\n", nsres);
> -
>    return nsres;
> - }
> -
> - if(NS_FAILED(nsres) || !nsdoc).
> -
>    return E_FAIL;
> -
> - nsIDOMDocument_QueryInterface(nsdoc, &IID_nsIDOMHTMLDocument, (void**)&nshtmldoc);
> - nsIDOMDocument_Release(nsdoc);
> -
> - nsAString_Init(&nsstr, NULL);
> -
> - nsres = nsIDOMHTMLDocument_GetTitle(nshtmldoc, &nsstr);
> - nsIDOMHTMLDocument_Release(nshtmldoc);
> -
> - if (NS_FAILED(nsres))
> -
>    return nsres;

Don't mix nsresult with HRESULT. Also ERR would be nice in this case.
> -
> - nsAString_GetData(&nsstr, &ret);
> - *p = SysAllocString(ret);

DOM API usually returns NULL instead of empty string. That's worth testing.
> -
> - return nsres;
>
> }

Again, don't mix nsresult with HRESULT.

Jacek