Fwd: [PATCH v2] ieframe: add implementation for WebBrowser_get_LocationName
Vijay Kiran Kamuju
infyquest at gmail.com
Mon Mar 4 11:58:45 CST 2019
Hi Jacek,
Can you send the tests with todo block?
The tests looks good to me.
If the url doesn't implement IHTMLDocument2, It should return the url.
Hence I was calling get_location_url at the start to cover the empty
string case, which is causing a leak.
eg:"about:blank"
The same holds true for null/empty string as input.
Meanwhile, I will see how I can improve the implementation.
Thanks,
Vijay
On Mon, Mar 4, 2019 at 5:56 PM Jacek Caban <jacek at codeweavers.com> wrote:
>
> Hi Vijay,
>
> On 3/1/19 2:58 AM, Vijay Kiran Kamuju wrote:
>
>
> +#define test_LocationName(a,b) _test_LocationName(__LINE__,a,b)
> +static void _test_LocationName(unsigned line, IWebBrowser2 *unk, const char *exname)
> +{
> + BSTR name = (void*)0xdeadbeef;
> + static const char wine_title[] = "WineHQ - Run Windows applications on Linux, BSD, Solaris and Mac OS X";
> + HRESULT hres;
> +
> + hres = IWebBrowser2_get_LocationName(wb, &name);
> + ok_(__FILE__,line) (hres == (*exname ? S_OK : S_FALSE), "get_LocationName failed: %08x\n", hres);
> + if (SUCCEEDED(hres))
> + {
> + ok_(__FILE__,line) (!strcmp_wa(name, exname) || !strcmp_wa(name,wine_title), "unexpected name: %s\n", wine_dbgstr_w(name));
>
>
> This || here makes the test pass even if you'd always simply return URL. See the attached patch for how it could be improved.
>
>
> diff --git a/dlls/ieframe/webbrowser.c b/dlls/ieframe/webbrowser.c
> index 03acf3886989..9e34cab51bdb 100644
> --- a/dlls/ieframe/webbrowser.c
> +++ b/dlls/ieframe/webbrowser.c
> @@ -503,8 +503,24 @@ static HRESULT WINAPI WebBrowser_put_Height(IWebBrowser2 *iface, LONG Height)
> static HRESULT WINAPI WebBrowser_get_LocationName(IWebBrowser2 *iface, BSTR *LocationName)
> {
> WebBrowser *This = impl_from_IWebBrowser2(iface);
> - FIXME("(%p)->(%p)\n", This, LocationName);
> - return E_NOTIMPL;
> + HRESULT hres;
> +
> + TRACE("(%p)->(%p)\n", This, LocationName);
> +
> + hres = get_location_url(&This->doc_host, LocationName);
> + if (This->doc_host.document)
> + {
> + IHTMLDocument2 *doc = NULL;
> + hres = IUnknown_QueryInterface(This->doc_host.document, &IID_IHTMLDocument2, (void **)&doc);
> + if (SUCCEEDED(hres))
> + {
> + IHTMLDocument2_get_title(doc, LocationName);
> + if (SysStringLen(*LocationName) == 0)
>
>
> You leak LocationName in this case (even an empty string is allocated).
>
>
> + hres = get_location_url(&This->doc_host, LocationName);
>
>
> What happens if have a document that doesn't implement IHTMLDocument2? I guess it should just return URL here, but it will fail with your patch.
>
>
> Thanks,
>
> Jacek
More information about the wine-devel
mailing list