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(a)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