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
- 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.