[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