Re: [PATCH] mshtml: Implemented numeral parsing in get_frame_by_name (try 2).

[Jacek Caban]

Hi Qian,

On 04/01/14 22:08, Qian Hong wrote:
> diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c
> index a57dfba..3d6323c 100644
> --- a/dlls/mshtml/htmlwindow.c
> +++ b/dlls/mshtml/htmlwindow.c
> @@ -379,6 +379,20 @@ HRESULT get_frame_by_name(HTMLOuterWindow *This, const WCHAR *name, BOOL deep, H

This is not the right place to do it. See callers of the function. I
don't think we want that behaviour in, for example, get_target_window.
> UINT32 length, i;
>      nsresult nsres;
>      HRESULT hres = S_OK;
> +    VARIANT v, idx;
> +
> +    V_VT(&v) = VT_BSTR;
> +    V_BSTR(&v) = SysAllocString(name);
> +    VariantInit(&idx);
> +    hres = VariantChangeType(&idx, &v, VARIANT_LOCALBOOL, VT_I4);

There are better ways to do that conversion, like strtolW.
> +    VariantClear(&v);
> +    if (SUCCEEDED(hres))
> +    {
> +        hres = get_frame_by_index(This, V_I4(&idx), ret);
> +        VariantClear(&idx);
> +        return hres;
> +    }
> +    VariantClear(&idx);

Clearing VARIANT of VT_I4 type is not really useful.

Also, as Austin said, this definitely needs a test case. You may write
that in JavaScript, if you prefer. See existing tests in *.html files.

Cheers,
Jacek

[PATCH] mshtml: Implemented numeral parsing in get_frame_by_name (try 2).

[Qian Hong]

Hi Jacek,

Thanks for comment, this patch is just a gift for April Day to make you laugh...

Actually there are two bugs in Wine's builtin iexplore, I work around
one of them and leave the other intentionally, and now every test
cases passes - actually passed without running at all :)

Unfortunately this joke looks too serious and so far nobody understand
it is a joke, it seems like the author of the bad joke should be
blamed... I appreciate for any advice to make a better joke in 2015 :)

We miss your improvement of HTML5 driver... lol

BTW, I don't have time to work on these two bugs right now, hopefully
they are interesting enough for you <grin>