Re: mshtml: Implement IHTMLStyle get/put posLeft (Try 7)

[Jacek Caban]

Alistair Leslie-Hughes wrote:
> Hi,
>   Clean up the get function.
>   Make sure we round down the value passed in.
>   Corrected test to prove above.
>
> Changelog:
>     mshtml: Implement IHTMLStyle get/put posLeft

+
+    V_VT(&v) = VT_I4;
+    V_I4(&v) = floor(f);
+
+    if(VariantChangeType(&v, &v, 0, VT_BSTR) == S_OK)


- It won't work for some locales
- It's probably better to use sprintfW for this and set units to px for free

+    hres = get_style_attr(This, STYLEID_LEFT, &ret);
+    if(SUCCEEDED(hres))
+    {
+        if(ret)
+            *p = atoiW(ret);
+    }

- Your patch adds support only for px values, so you should check if we 
really return px value. Something like

for(ptr=ret; isdigitW(*ptr); ptr++);
if(*ptr || strcmp(ptr, pxW)) {
    FIXME(...);
    return E_NOTIMPL;
}

should be fine.

- You leak ret
- It's better to use get_nsstyle_attr_nsval to avoid one useless memory 
allocation.


And you really want to move these function to separated functions like 
[get|set]_style_pos that would take styleid_t as an argument to avoid 
code duplication.


Jacek

mshtml: Implement IHTMLStyle get/put posLeft (Try 7)

[Jacek Caban]

Jacek Caban wrote:
> for(ptr=ret; isdigitW(*ptr); ptr++);
> if(*ptr || strcmp(ptr, pxW)) {

.. && .. of course...


Jacek