Re: mshtml: Added IHTMLTableRow::insertCell method implementation. (try 2)

[Jacek Caban]

Hi Zhenbo,

On 05/23/14 14:20, Zhenbo Li wrote:
> iff --git a/dlls/mshtml/htmltablerow.c b/dlls/mshtml/htmltablerow.c
> index 731b403..f1b672b 100644
> --- a/dlls/mshtml/htmltablerow.c
> +++ b/dlls/mshtml/htmltablerow.c
> @@ -308,8 +308,27 @@ static HRESULT WINAPI HTMLTableRow_get_cells(IHTMLTableRow *iface, IHTMLElementC
>  static HRESULT WINAPI HTMLTableRow_insertCell(IHTMLTableRow *iface, LONG index, IDispatch **row)
>  {
>      HTMLTableRow *This = impl_from_IHTMLTableRow(iface);
> -    FIXME("(%p)->(%d %p)\n", This, index, row);
> -    return E_NOTIMPL;
> +    nsIDOMHTMLElement *nselem;
> +    HTMLElement *elem = NULL;

You shouldn't need to initialize it here.
> +    nsresult nsres;
> +    HRESULT hres;
> +
> +    TRACE("(%p)->(%d %p)\n", This, index, row);
> +    nsres = nsIDOMHTMLTableRowElement_InsertCell(This->nsrow, index, &nselem);
> +    if(NS_FAILED(nsres)) {
> +        ERR("Insert Cell at %d failed: %08x\n", index, nsres);
> +        return E_FAIL;
> +    }
> +
> +    hres = HTMLTableCell_Create(This->element.node.doc, nselem, &elem);
> +    if (FAILED(hres)) {
> +        ERR("Create TableCell failed: %08x\n", hres);
> +        return hres;
> +    }
> +    nsIDOMHTMLElement_Release(nselem);

There it a leak in error case. Move Release call before if statement.
> +
> +    hres = IHTMLElement_QueryInterface(&elem->IHTMLElement_iface, &IID_IDispatch, (void **)row);
> +    return hres;

You don't need QueryInterface here. Cast is fine as long as it's done on
an interface inheriting from IDispatch (but not on object, as you did in
previous version of the patch).

Cheers,
Jacek