[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH v11 06/10] mshtml: Associate props with attributes for IE8 mode, but keep them stringifie
From:       Jacek Caban <jacek () codeweavers ! com>
Date:       2021-11-30 20:59:28
Message-ID: 410b0250-676d-2bc1-3d92-6d464af9c298 () codeweavers ! com
[Download RAW message or body]

Hi Gabriel,

On 11/29/21 5:31 PM, Gabriel Ivăncescu wrote:
> +static inline WCHAR *translate_attr_name(WCHAR *attr_name, compat_mode_t compat_mode)
> +{
> +    WCHAR *ret = attr_name;
> +
> +    if(compat_mode >= COMPAT_MODE_IE8 && !wcsicmp(attr_name, L"class"))
> +        ret = (WCHAR*)L"className";
> +    return ret;
> +}


Please avoid this cast.


> +                if(get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
> +                    VariantClear(AttributeValue);
> +                    V_VT(AttributeValue) = VT_NULL;
> +                    return S_OK;
> +                }


It's not right in general, see the attached test. I'm not expecting that 
you implement all those IE8-specific things (in fact, I don't think we 
need to care too much about it), but the patch in its current shape can 
break things. Could you just remove this if() and live todo_wines?


Thanks,

Jacek


["patch.diff" (text/x-patch)]

diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 2ef9173c095..bd39057a80e 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1143,6 +1143,10 @@ sync_test("elem_attr", function() {
     ok(r === (v < 9 ? true : undefined), "testattr removeAttribute returned " + r);
     ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr = " + elem.testattr);
 
+    elem.disabled = true;
+    r = elem.getAttribute("disabled");
+    ok(r === (v < 8 ? true : (v < 9 ? "disabled" : "")), "disabled = " + r);
+
     arr.toString = function() { return 42; }
     elem.testattr = arr;
     r = elem.getAttribute("testattr");


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic