Re: mshtml: Added IHTMLTable::width property implementation.

[Jacek Caban]

Hi Zhenbo,

It looks better now.

On 04/19/14 03:38, Zhenbo Li wrote:
> @@ -395,15 +422,60 @@ static HRESULT WINAPI HTMLTable_get_rows(IHTMLTable *iface, IHTMLElementCollecti
>  static HRESULT WINAPI HTMLTable_put_width(IHTMLTable *iface, VARIANT v)
>  {
>      HTMLTable *This = impl_from_IHTMLTable(iface);
> -    FIXME("(%p)->(%s)\n", This, debugstr_variant(&v));
> -    return E_NOTIMPL;
> +    nsAString val;
> +    HRESULT hres;
> +    nsresult nsres;
> +
> +    TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));
> +    hres = var2str(&val, &v);
> +
> +    if (hres != S_OK){
> +        ERR("Set Width(%s) failed when initializing a nsAString!\n",
> +            debugstr_variant(&v));
> +        nsAString_Finish(&val);

val is not initialized here, so you shouldn't call nsAString_Finish on it.
> +        return hres;
> +    }
> +
> +    nsres = nsIDOMHTMLTableElement_SetWidth(This->nstable, &val);
> +    nsAString_Finish(&val);
> +
> +    if (NS_FAILED(nsres)){
> +        ERR("Set Width(%s) failed!\n", debugstr_variant(&v));
> +        return E_FAIL;
> +    }
> +
> +    return S_OK;
>  }
>
> static HRESULT WINAPI HTMLTable_get_width(IHTMLTable *iface, VARIANT *p)
>  {
>      HTMLTable *This = impl_from_IHTMLTable(iface);
> -    FIXME("(%p)->(%p)\n", This, p);
> -    return E_NOTIMPL;
> +    nsAString val;
> +    nsresult nsres;
> +    DOUBLE width;
> +    const PRUnichar *str;
> +
> +    TRACE("(%p)->(%p)\n", This, p);
> +    nsAString_Init(&val, NULL);
> +    nsres = nsIDOMHTMLTableElement_GetWidth(This->nstable, &val);
> +    if (NS_FAILED(nsres)){
> +        ERR("Get Width(%s) failed!\n", debugstr_variant(p));
> +        nsAString_Finish(&val);
> +        return E_FAIL;
> +    }
> +
> +    nsAString_GetData(&val, &str);
> +    nsres = VarR8FromStr(str, 0, 0, &width);
> +    if (nsres == S_OK){
> +        V_VT(p) = VT_I4;
> +        V_I4(p) = (LONG)width;
> +    } else {            /* Symbols are in this string */
> +        V_VT(p) = VT_BSTR;
> +        V_BSTR(p) = SysAllocString(str);
> +        if (!V_BSTR(p))
> +            return E_OUTOFMEMORY;
> +    }
> +    return S_OK;
>  }
>
> static HRESULT WINAPI HTMLTable_put_height(IHTMLTable *iface, VARIANT v)
> diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c
> index 23677f4..10aca6c 100644
> --- a/dlls/mshtml/tests/dom.c
> +++ b/dlls/mshtml/tests/dom.c
> @@ -5721,6 +5721,25 @@ static void _test_table_cell_spacing(unsigned line, IHTMLTable *table, const cha
>      VariantClear(&v);
>  }
>
> +#define cmp_length(a,b) _cmp_length(__LINE__,a,b)
> +static void _cmp_length(unsigned line, VARIANT got, const char * exstr)
> +{
> +    static char buf[64];
> +    ok_(__FILE__,line) (V_VT(&got)==VT_BSTR || V_VT(&got)==VT_I4,
> +                        "V_VT is %hu, expected VT_BSTR(%hu) or VT_I4(%hu)\n",
> +                        V_VT(&got), VT_BSTR, VT_I4);

Please make it stricter and allow only one, exactly expected in given
case, type. According to my quick tests (as I mentioned earlier), it's
always VT_BSTR. Did you ever see it being VT_I4 in real Windows? If my
testing is tight, this test hides a problem with your getter implementation.


Cheers,
Jacek

mshtml: Added IHTMLTable::width property implementation.

[Qian Hong]

> + nsres = VarR8FromStr(str, 0, 0, &width);
> + if (nsres == S_OK){

Also it's not correct or at least not good to use nsresult  sres here,
HRESULT hres is better, I don't think we want to share the same
variable for nsresult and HRESULT type.



-- 
Regards,
Qian Hong

-
http://www.winehq.org