On 04/25/14 15:39, Zhenbo Li wrote:
+ 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. val is initialized in var2str. Should I make it more explicit?
No, it's not initialized in error case.
+ 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. Yes, such danger exists. I'll find a proper way to solve it.
+ 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; } Thank you for mentioning that, but I don't think this helper works. width property can have two types: a number, or a percentage. number(like 11.9) would be truncated to 11 percentage("40.1%") won't be truncated. (Test case proved that)
So this helper would cause error. How about check if the string has the symbol '%', then to decide whether to call nsstr_to_rounded_bstr?
Well, this helper takes care of that. It will leave percent value untouched, what's what the second loop and if is for. Why do you think this won't work? Jacek