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