[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 <gabrielopcode at gmail.com>
> ---
>
> 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