Re: [PATCH (try2)] ieframe: Strip 'file://' from file URLs in BEFORENAVIGATE2 callbacks

[Jacek Caban]

Hi Andrew,

On 01/09/13 16:56, Andrew Eikum wrote:
> This should fix the Office 2007 half of Bug 17271.
>
> try2: Fix test failures on XP by expanding the expected path name.
>
> dlls/ieframe/navigate.c         |  7 +++++-
>  dlls/ieframe/tests/webbrowser.c | 56 ++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 59 insertions(+), 4 deletions(-)
>
> 0001-ieframe-Strip-file-from-file-URLs-in-BEFORENAVIGATE2.patch
>
> diff --git a/dlls/ieframe/navigate.c b/dlls/ieframe/navigate.c
> index dd4a976..2601ba8 100644
> --- a/dlls/ieframe/navigate.c
> +++ b/dlls/ieframe/navigate.c
> @@ -566,6 +566,8 @@ static void on_before_navigate2(DocHost *This, LPCWSTR url, SAFEARRAY *post_data
>      VARIANT var_url, var_flags, var_frame_name, var_post_data, var_post_data2, var_headers;
>      DISPPARAMS dispparams;
>      VARIANTARG params[7];
> -
>
> WCHAR file_path[MAX_PATH];
> -
>
> DWORD file_path_len = sizeof(file_path) / sizeof(*file_path);
>
> dispparams.cArgs = 7;
> dispparams.cNamedArgs = 0;
>
> @@ -607,7 +609,10 @@ static void on_before_navigate2(DocHost *This, LPCWSTR url, SAFEARRAY *post_data
>      V_VT(params+5) = (VT_BYREF|VT_VARIANT);
>      V_VARIANTREF(params+5) = &var_url;
>      V_VT(&var_url) = VT_BSTR;
> - V_BSTR(&var_url) = SysAllocString(url);
> -
>
> if(PathCreateFromUrlW(url, file_path, &file_path_len, 0) == S_OK)
> -
>    V_BSTR(&var_url) = SysAllocString(file_path);
> -
>
> else
> -
>    V_BSTR(&var_url) = SysAllocString(url);
>
> V_VT(params+6) = (VT_DISPATCH);
>    V_DISPATCH(params+6) = (IDispatch*)This->wb;
>
> diff --git a/dlls/ieframe/tests/webbrowser.c b/dlls/ieframe/tests/webbrowser.c
> index 25cce8a..70c785b 100644
> --- a/dlls/ieframe/tests/webbrowser.c
> +++ b/dlls/ieframe/tests/webbrowser.c
> @@ -151,7 +151,7 @@ static VARIANT_BOOL exvb;
>  static IWebBrowser2 *wb;
>
> static HWND container_hwnd, shell_embedding_hwnd;
> -static BOOL is_downloading, is_first_load, use_container_olecmd, test_close, is_http, use_container_dochostui;
> +static BOOL is_downloading, is_first_load, use_container_olecmd, test_close, is_http, is_file, use_container_dochostui;
>  static HRESULT hr_dochost_TranslateAccelerator = E_NOTIMPL;
>  static HRESULT hr_site_TranslateAccelerator = E_NOTIMPL;
>  static const char *current_url;
> @@ -690,8 +690,14 @@ static void test_OnBeforeNavigate(const VARIANT *disp, const VARIANT *url, const
>          ok(V_VT(V_VARIANTREF(url)) == VT_BSTR, "V_VT(V_VARIANTREF(url))=%d, expected VT_BSTR\n",
>             V_VT(V_VARIANTREF(url)));
>          ok(V_BSTR(V_VARIANTREF(url)) != NULL, "V_BSTR(V_VARIANTREF(url)) == NULL\n");
> -
>    ok(!strcmp_wa(V_BSTR(V_VARIANTREF(url)), current_url), "unexpected url %s, expected %s\n",
> -
>       wine_dbgstr_w(V_BSTR(V_VARIANTREF(url))), current_url);
> -
>    if(is_file){
> -
>        char full_path[MAX_PATH];
> -
>        GetLongPathNameA(current_url, full_path, sizeof(full_path));

I think expanding the path would be better done while constructing the URL instead of here.

+    test_DoVerb(webbrowser);
+    test_Navigate2(webbrowser, file_url);
+
+    IWebBrowser2_Release(webbrowser);

This is not enough for proper clean up. You may verify that by checking ref count here. At least setting client site to NULL is needed.

Cheers,
Jacek