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

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

[Ivan Sinitsin]

В сообщении от Friday 04 July 2008 19:23:32 вы написали:
> 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.

Hi, 

I make test for HTMLDocument_(Get|Set)Title.
What do you think about it? Is it right?

On IE it - ok, but Gecko - failed.
> Jacek

--
Sinitsin Ivan