On 04/22/14 17:30, Zhenbo Li wrote:
> diff --git a/dlls/mshtml/htmltable.c b/dlls/mshtml/htmltable.c
> index d711ed7..e16aee2 100644
> --- a/dlls/mshtml/htmltable.c
> +++ b/dlls/mshtml/htmltable.c
> @@ -57,6 +57,33 @@ static inline HTMLTable *impl_from_IHTMLTable3(IHTMLTable3 *iface)
> return CONTAINING_RECORD(iface, HTMLTable, IHTMLTable3_iface);
> }
>
> +static HRESULT var2str(nsAString *nsstr, const VARIANT *p)
> +{
Nit: please swap arguments order.
> + BSTR str;
> + BOOL ret;
> +
> + switch(V_VT(p)) {
> + case VT_BSTR:
> + return nsAString_Init(nsstr, V_BSTR(p))?
> + S_OK : E_FAIL;
E_OUTOFMEMORY would be more appropriate here.
> + case VT_R8:
> + VarBstrFromR8(V_R8(p), 0, 0, &str);
> + break;
> + case VT_R4:
> + VarBstrFromR4(V_R4(p), 0, 0, &str);
> + break;
> + case VT_I4:
> + VarBstrFromI4(V_I4(p), 0, 0, &str);
> + break;
> + default:
> + FIXME("unsupported arg %s\n", debugstr_variant(p));
> + return E_NOTIMPL;
> + }
> + ret = nsAString_Init(nsstr, str);
> + SysFreeString(str);
> + return ret ? S_OK : E_FAIL;
> +}
Same here.
> +
> static HRESULT WINAPI HTMLTable_QueryInterface(IHTMLTable *iface,
> REFIID riid, void **ppv)
> {
> @@ -395,15 +422,79 @@ 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, you can'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;
> + const PRUnichar *str;
> + BSTR bstr;
> + DOUBLE width;
> + nsresult nsres;
> + HRESULT hres;
> +
> + 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);
> + hres = VarR8FromStr(str, 0, 0, &width);
> +
> + /* WinAPI would return VT_BSTR, not VT_I4 */
> + switch(hres) {
> + case S_OK: /* Truncate that number */
> + hres = VarBstrFromI4((LONG)width, 0, 0, &bstr);
There may be overflow here.
> + if (hres != S_OK) {
> + ERR("VarBstrFromI4 failed, err = %d\n", hres);
> + return E_FAIL;
> + }
> + V_VT(p) = VT_BSTR;
> + V_BSTR(p) = bstr;
> + break;
> + case DISP_E_TYPEMISMATCH: /* Has symbols */
> + V_VT(p) = VT_BSTR;
> + V_BSTR(p) = SysAllocString(str);
> + if (!V_BSTR(p)) {
> + ERR("SysAllocString(%s) failed!\n", debugstr_w(str));
> + return E_FAIL;
> + }
> + break;
> + default:
> + ERR("VarR8FromStr(%s) failed, err = %d\n", debugstr_w(str), hres);
> + return E_FAIL;
I find it more complicated than it needs to be. You don't really need
full conversion here, just a simple string parsing. How about using such
helper:
static HRESULT nsstr_to_rounded_bstr(const nsAString *nsstr, BSTR *ret_ptr)
{
const PRUnichar *str, *ptr, *end = NULL;
BSTR ret;
nsAString_GetData(nsstr, &str);
for(ptr = str; isdigitW(*ptr); ptr++);
if(*ptr == '.') {
for(end = ptr++; isdigitW(*ptr); ptr++);
if(*ptr)
end = NULL;
}
ret = end ? SysAllocStringLen(str, end-str) : SysAllocString(str);
if(!ret)
return E_OUTOFMEMORY;
*ret_ptr = ret;
return S_OK;
}
Cheers,
Jacek