From: Zhenbo Li Subject: mshtml: Added IHTMLTable::width property implementation. (try 3) Message-Id: <53568B14.2080806@gmail.com> Date: Tue, 22 Apr 2014 23:30:28 +0800 Thank you for checking my patch. try 3: 2014-04-21 17:30 GMT+08:00 Jacek Caban : > val is not initialized here, so you shouldn't call nsAString_Finish on it. I initialized val in var2str(). Maybe I should move that code to setter? > 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. You're right. I should follow test result, instead of MSDN. 2014-04-21 22:42 GMT+08:00 Qian Hong : > Also it's not correct or at least not good to use nsresult sres here, Sorry, I forgot to fix that... I have a question then: As I use a switch to truncate the number in getter, getter seems to be too big. Should I move that code to another helper? Or do that when implementing other properties like height? try 2: Thanks for Jacek's suggestions. 1. Convert float to int in getter, instead of setter. 2. Make the helper more generic (var2str) 3. Don't mistake HRESULT/nsresult 4. Don't use return_nsst in the old way 5. More strict in cmp_length origin: As my test case shows, windows would truncate float numbers. I considered VarInt() in oleaut32.dll, but I think to declare var2str_length() is more clear. --- dlls/mshtml/htmltable.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++-- dlls/mshtml/tests/dom.c | 48 ++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 4 deletions(-) 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) +{ + BSTR str; + BOOL ret; + + switch(V_VT(p)) { + case VT_BSTR: + return nsAString_Init(nsstr, V_BSTR(p))? + S_OK : E_FAIL; + 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; +} + 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); + 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); + 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; + } + + + 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 65ca1d3..6562864 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -5883,6 +5883,54 @@ static void test_table_elem(IHTMLElement *elem) ok(hres == S_OK, "put_bgColor failed: %08x\n", hres); VariantClear(&vDefaultbg); + V_VT(&v) = VT_BSTR; + V_BSTR(&v) = a2bstr("11"); + hres = IHTMLTable_put_width(table, v); + ok(hres == S_OK, "put_width = %08x\n", hres); + VariantClear(&v); + IHTMLTable_get_width(table, &v); + ok(hres == S_OK, "get_width = %08x\n", hres); + ok(!strcmp_wa(V_BSTR(&v), "11"), "Expected 11, got %s\n", wine_dbgstr_w(V_BSTR(&v))); + VariantClear(&v); + + V_VT(&v) = VT_BSTR; + V_BSTR(&v) = a2bstr("11.9"); + hres = IHTMLTable_put_width(table, v); + ok(hres == S_OK, "put_width = %08x\n", hres); + VariantClear(&v); + IHTMLTable_get_width(table, &v); + ok(hres == S_OK, "get_width = %08x\n", hres); + ok(!strcmp_wa(V_BSTR(&v), "11"), "Expected 11, got %s\n", wine_dbgstr_w(V_BSTR(&v))); + VariantClear(&v); + + V_VT(&v) = VT_BSTR; + V_BSTR(&v) = a2bstr("40.2%"); + hres = IHTMLTable_put_width(table, v); + ok(hres == S_OK, "put_width = %08x\n", hres); + VariantClear(&v); + IHTMLTable_get_width(table, &v); + ok(hres == S_OK, "get_width = %08x\n", hres); + ok(!strcmp_wa(V_BSTR(&v), "40.2%"), "Expected 40.2%%, got %s\n", wine_dbgstr_w(V_BSTR(&v))); + VariantClear(&v); + + V_VT(&v) = VT_I4; + V_I4(&v) = 11; + hres = IHTMLTable_put_width(table, v); + ok(hres == S_OK, "put_width = %08x\n", hres); + IHTMLTable_get_width(table, &v); + ok(hres == S_OK, "get_width = %08x\n", hres); + ok(!strcmp_wa(V_BSTR(&v), "11"), "Expected 11, got %s\n", wine_dbgstr_w(V_BSTR(&v))); + VariantClear(&v); + + V_VT(&v) = VT_R8; + V_R8(&v) = 11.9; + hres = IHTMLTable_put_width(table, v); + ok(hres == S_OK, "put_width = %08x\n", hres); + IHTMLTable_get_width(table, &v); + ok(hres == S_OK, "get_width = %08x\n", hres); + ok(!strcmp_wa(V_BSTR(&v), "11"), "Expected 11, got %s\n", wine_dbgstr_w(V_BSTR(&v))); + VariantClear(&v); + IHTMLTable_Release(table); }