Re: [PATCH v10 2/5] mshtml: Fix setting and retrieving attributes in IE8 mode.

[Jacek Caban]

On 11/29/21 2:14 PM, Gabriel Ivăncescu wrote:
> For non-builtin props, getAttribute retrieves the stringified value of the
> prop. For builtins, however, getAttribute returns null unless they were
> set to a string. setAttribute also stringifies the value if it's a builtin.
> Signed-off-by: Gabriel Ivăncescu
>
> I'm testing for/htmlFor since prototype.js also uses it for translations,
> but it doesn't seem to be affected here.
>
> dlls/mshtml/htmlelem.c            | 193 ++++++++++++++++++++----------
>   dlls/mshtml/tests/documentmode.js |  76 +++++++++---
>   2 files changed, 188 insertions(+), 81 deletions(-)
>
> diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
> index 3f7c60b..bd9dc29 100644
> --- a/dlls/mshtml/htmlelem.c
> +++ b/dlls/mshtml/htmlelem.c
> @@ -1067,6 +1067,15 @@ static HRESULT WINAPI HTMLElement_Invoke(IHTMLElement *iface, DISPID dispIdMembe
>               wFlags, pDispParams, pVarResult, pExcepInfo, puArgErr);
>   }
>
> +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;
>
> +}
> - static HRESULT set_elem_attr_value_by_dispid(HTMLElement *elem, DISPID dispid, VARIANT *v)
> {
>     DISPID propput_dispid = DISPID_PROPERTYPUT;
>
> @@ -1086,34 +1095,62 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
>                                                  VARIANT AttributeValue, LONG lFlags)
>   {
>       HTMLElement *This = impl_from_IHTMLElement(iface);
> -
>
> compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
> -
>
> nsAString name_str, value_str;
> -
>
> VARIANT val = AttributeValue;
> -
>
> BOOL needs_free = FALSE;
> -
>
> nsresult nsres;
>  DISPID dispid;
>  HRESULT hres;
>
> TRACE("(%p)->(%s %s %08x)\n", This, debugstr_w(strAttributeName), debugstr_variant(&AttributeValue), lFlags);
> - if(This->dom_element && dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
> -
>    nsAString name_str, value_str;
> -
>    nsresult nsres;
> -
> -
>    hres = variant_to_nsstr(&AttributeValue, FALSE, &value_str);
> - if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {

Why do you change IE9+ code path? Would changing the condition from IE8 
to IE9 be enough for this code path?
> static HRESULT WINAPI HTMLElement_put_className(IHTMLElement *iface, BSTR v)
> @@ -6463,6 +6523,9 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex)
>       nsresult nsres;
>       HRESULT hres;
> - if(dispex_compat_mode(dispex) >= COMPAT_MODE_IE9)
> -
>    return S_OK;

While the rest of the patch could be split as well, this part (while 
probably correct) definitely doesn't belong to this patch.

Thanks,

Jacek