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

[Nikolay Sivov]

On 4/6/2014 18:48, Zhenbo Li wrote:
> try 2:
> Don't free a string too early.
>
> ---
>  dlls/mshtml/htmltable.c | 41 +++++++++++++++++++++++++++++++++++++----
>  dlls/mshtml/tests/dom.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 4 deletions(-)

Also please check for variant_to_nscolor() return value.

mshtml: Added IHTMLTable::bgColor property implementation.

[Zhenbo Li]

2014-04-07 13:05 GMT+08:00 Nikolay Sivov <bunglehead(a)gmail.com>:
> On 4/6/2014 18:48, Zhenbo Li wrote:
> > try 2:
> > Don't free a string too early.
> >
> > ---
> >  dlls/mshtml/htmltable.c | 41 +++++++++++++++++++++++++++++++++++++----
> >  dlls/mshtml/tests/dom.c | 35 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 72 insertions(+), 4 deletions(-)
>
> Also please check for variant_to_nscolor() return value.

Thanks, Nikolay. Sorry for my late reply.
I've checked the previous code in htmlbody.c, and found such code

385 static HRESULT WINAPI HTMLBodyElement_put_bgColor(IHTMLBodyElement
*iface, VARIANT v)
386 {
387     HTMLBodyElement *This = impl_from_IHTMLBodyElement(iface);
388     nsAString strColor;
389     nsresult nsres;
390
391     TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));
392
393     if(!variant_to_nscolor(&v, &strColor))
394         return S_OK;
395
396     nsres = nsIDOMHTMLBodyElement_SetBgColor(This->nsbody, &strColor);
397     nsAString_Finish(&strColor);
398     if(NS_FAILED(nsres))
399         ERR("SetBgColor failed: %08x\n", nsres);
400
401     return S_OK;
402 }

So when variant_to_nscolor failed (because VARIANT is invalid), wine
would ignore it,
and return S_OK. Maybe return E_FAIL is better?

Also, in variant_to_nscolor, the return value of nsAString_Init is
ignored. Is it good?
Or should I write a patch to fix it?

Thank you.
-- 
Have a nice day!
Zhenbo Li