[PATCH 0/7] MR8251: mshtml: getAttributeNode and removal fixes.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlattr.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/dlls/mshtml/htmlattr.c b/dlls/mshtml/htmlattr.c index 1a95c1b482e..ff6173aacdb 100644 --- a/dlls/mshtml/htmlattr.c +++ b/dlls/mshtml/htmlattr.c @@ -87,9 +87,9 @@ static HRESULT WINAPI HTMLDOMAttribute_get_nodeValue(IHTMLDOMAttribute *iface, V static HRESULT WINAPI HTMLDOMAttribute_get_specified(IHTMLDOMAttribute *iface, VARIANT_BOOL *p) { HTMLDOMAttribute *This = impl_from_IHTMLDOMAttribute(iface); - nsIDOMAttr *nsattr; nsAString nsname; nsresult nsres; + cpp_bool r; TRACE("(%p)->(%p)\n", This, p); @@ -105,19 +105,10 @@ static HRESULT WINAPI HTMLDOMAttribute_get_specified(IHTMLDOMAttribute *iface, V /* FIXME: This is not exactly right, we have some attributes that don't map directly to Gecko attributes. */ nsAString_InitDepend(&nsname, dispex_builtin_prop_name(&This->elem->node.event_target.dispex, This->dispid)); - nsres = nsIDOMElement_GetAttributeNode(This->elem->dom_element, &nsname, &nsattr); + nsres = nsIDOMElement_HasAttribute(This->elem->dom_element, &nsname, &r); nsAString_Finish(&nsname); - if(NS_FAILED(nsres)) - return E_FAIL; - /* If the Gecko attribute node can be found, we know that the attribute is specified. - There is no point in calling GetSpecified */ - if(nsattr) { - nsIDOMAttr_Release(nsattr); - *p = VARIANT_TRUE; - }else { - *p = VARIANT_FALSE; - } + *p = variant_bool(NS_SUCCEEDED(nsres) && r); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Surprisingly, this applies even in IE9+ modes when using IHTMLElement4's methods, but we can't test that yet because it's too broken in wine. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 152 +++++++++++++++++++++++++++++++++++++++- dlls/mshtml/tests/dom.c | 59 ++++++++++++++++ 2 files changed, 210 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index fcc6394c0d2..621bc705da3 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -7677,6 +7677,153 @@ static HRESULT create_filters_collection(compat_mode_t compat_mode, IHTMLFilters return S_OK; } +static inline BOOL is_valid_attr_dispid(DISPID id) +{ + /* Builtin props that can be treated as attributes; MUST be sorted by underlying dispid value. */ + static const DISPID attr_dispids[] = { + DISPID_IHTMLELEMENT2_TABINDEX, + DISPID_IHTMLELEMENT_TITLE, + DISPID_IHTMLELEMENT_STYLE, + DISPID_IHTMLELEMENT3_DISABLED, + DISPID_IHTMLELEMENT_ID, + DISPID_IHTMLELEMENT2_ACCESSKEY, + DISPID_IHTMLELEMENT_LANG, + DISPID_IHTMLELEMENT_LANGUAGE, + DISPID_IHTMLELEMENT2_DIR, + DISPID_IHTMLELEMENT3_CONTENTEDITABLE, + DISPID_IHTMLELEMENT3_HIDEFOCUS, + DISPID_IHTMLELEMENT7_SPELLCHECK, + DISPID_IHTMLELEMENT_ONMOUSEOVER, + DISPID_IHTMLELEMENT_ONMOUSEOUT, + DISPID_IHTMLELEMENT_ONMOUSEDOWN, + DISPID_IHTMLELEMENT_ONMOUSEUP, + DISPID_IHTMLELEMENT_ONMOUSEMOVE, + DISPID_IHTMLELEMENT_ONKEYDOWN, + DISPID_IHTMLELEMENT_ONKEYUP, + DISPID_IHTMLELEMENT_ONKEYPRESS, + DISPID_IHTMLELEMENT_ONCLICK, + DISPID_IHTMLELEMENT_ONDBLCLICK, + DISPID_IHTMLELEMENT6_ONSELECT, + DISPID_IHTMLELEMENT6_ONSUBMIT, + DISPID_IHTMLELEMENT6_ONRESET, + DISPID_IHTMLELEMENT_ONHELP, + DISPID_IHTMLELEMENT2_ONFOCUS, + DISPID_IHTMLELEMENT2_ONBLUR, + DISPID_IHTMLELEMENT_ONROWEXIT, + DISPID_IHTMLELEMENT_ONROWENTER, + DISPID_IHTMLELEMENT_ONBEFOREUPDATE, + DISPID_IHTMLELEMENT_ONAFTERUPDATE, + DISPID_IHTMLELEMENT2_ONREADYSTATECHANGE, + DISPID_IHTMLELEMENT6_ONABORT, + DISPID_IHTMLELEMENT6_ONERROR, + DISPID_IHTMLELEMENT6_ONCHANGE, + DISPID_IHTMLELEMENT2_ONSCROLL, + DISPID_IHTMLELEMENT6_ONLOAD, + DISPID_IHTMLELEMENT_ONDRAGSTART, + DISPID_IHTMLELEMENT2_ONRESIZE, + DISPID_IHTMLELEMENT_ONSELECTSTART, + DISPID_IHTMLELEMENT_ONERRORUPDATE, + DISPID_IHTMLELEMENT_ONDATASETCHANGED, + DISPID_IHTMLELEMENT_ONDATAAVAILABLE, + DISPID_IHTMLELEMENT_ONDATASETCOMPLETE, + DISPID_IHTMLELEMENT_ONFILTERCHANGE, + DISPID_IHTMLELEMENT2_ONLOSECAPTURE, + DISPID_IHTMLELEMENT2_ONPROPERTYCHANGE, + DISPID_IHTMLELEMENT2_ONDRAG, + DISPID_IHTMLELEMENT2_ONDRAGEND, + DISPID_IHTMLELEMENT2_ONDRAGENTER, + DISPID_IHTMLELEMENT2_ONDRAGOVER, + DISPID_IHTMLELEMENT2_ONDRAGLEAVE, + DISPID_IHTMLELEMENT2_ONDROP, + DISPID_IHTMLELEMENT2_ONCUT, + DISPID_IHTMLELEMENT2_ONCOPY, + DISPID_IHTMLELEMENT2_ONPASTE, + DISPID_IHTMLELEMENT2_ONBEFORECUT, + DISPID_IHTMLELEMENT2_ONBEFORECOPY, + DISPID_IHTMLELEMENT2_ONBEFOREPASTE, + DISPID_IHTMLELEMENT2_ONROWSDELETE, + DISPID_IHTMLELEMENT2_ONROWSINSERTED, + DISPID_IHTMLELEMENT2_ONCELLCHANGE, + DISPID_IHTMLELEMENT2_ONCONTEXTMENU, + DISPID_IHTMLELEMENT2_ONBEFOREEDITFOCUS, + DISPID_IHTMLELEMENT3_ONLAYOUTCOMPLETE, + DISPID_IHTMLELEMENT3_ONPAGE, + DISPID_IHTMLELEMENT4_ONMOUSEWHEEL, + DISPID_IHTMLELEMENT3_ONBEFOREDEACTIVATE, + DISPID_IHTMLELEMENT3_ONMOVE, + DISPID_IHTMLELEMENT3_ONCONTROLSELECT, + DISPID_IHTMLELEMENT3_ONMOVESTART, + DISPID_IHTMLELEMENT3_ONMOVEEND, + DISPID_IHTMLELEMENT3_ONRESIZESTART, + DISPID_IHTMLELEMENT3_ONRESIZEEND, + DISPID_IHTMLELEMENT3_ONMOUSELEAVE, + DISPID_IHTMLELEMENT3_ONACTIVATE, + DISPID_IHTMLELEMENT3_ONDEACTIVATE, + DISPID_IHTMLELEMENT4_ONBEFOREACTIVATE, + DISPID_IHTMLELEMENT4_ONFOCUSIN, + DISPID_IHTMLELEMENT4_ONFOCUSOUT, + DISPID_IHTMLELEMENT6_ONINPUT, + DISPID_IHTMLELEMENT6_ONCANPLAY, + DISPID_IHTMLELEMENT6_ONCANPLAYTHROUGH, + DISPID_IHTMLELEMENT6_ONDURATIONCHANGE, + DISPID_IHTMLELEMENT6_ONEMPTIED, + DISPID_IHTMLELEMENT6_ONENDED, + DISPID_IHTMLELEMENT6_ONLOADEDDATA, + DISPID_IHTMLELEMENT6_ONLOADEDMETADATA, + DISPID_IHTMLELEMENT6_ONLOADSTART, + DISPID_IHTMLELEMENT6_ONPAUSE, + DISPID_IHTMLELEMENT6_ONPLAY, + DISPID_IHTMLELEMENT6_ONPLAYING, + DISPID_IHTMLELEMENT6_ONPROGRESS, + DISPID_IHTMLELEMENT6_ONRATECHANGE, + DISPID_IHTMLELEMENT6_ONSEEKED, + DISPID_IHTMLELEMENT6_ONSEEKING, + DISPID_IHTMLELEMENT6_ONSTALLED, + DISPID_IHTMLELEMENT6_ONSUSPEND, + DISPID_IHTMLELEMENT6_ONTIMEUPDATE, + DISPID_IHTMLELEMENT6_ONVOLUMECHANGE, + DISPID_IHTMLELEMENT6_ONWAITING, + DISPID_IHTMLELEMENT7_ONMSPOINTERDOWN, + DISPID_IHTMLELEMENT7_ONMSPOINTERMOVE, + DISPID_IHTMLELEMENT7_ONMSPOINTERUP, + DISPID_IHTMLELEMENT7_ONMSPOINTEROVER, + DISPID_IHTMLELEMENT7_ONMSPOINTEROUT, + DISPID_IHTMLELEMENT7_ONMSPOINTERCANCEL, + DISPID_IHTMLELEMENT7_ONMSPOINTERHOVER, + DISPID_IHTMLELEMENT7_ONMSGESTURESTART, + DISPID_IHTMLELEMENT7_ONMSGESTURECHANGE, + DISPID_IHTMLELEMENT7_ONMSGESTUREEND, + DISPID_IHTMLELEMENT7_ONMSGESTUREHOLD, + DISPID_IHTMLELEMENT7_ONMSGESTURETAP, + DISPID_IHTMLELEMENT7_ONMSGESTUREDOUBLETAP, + DISPID_IHTMLELEMENT7_ONMSINERTIASTART, + DISPID_IHTMLELEMENT7_ONMSLOSTPOINTERCAPTURE, + DISPID_IHTMLELEMENT7_ONMSGOTPOINTERCAPTURE, + DISPID_IHTMLELEMENT7_ONMSTRANSITIONSTART, + DISPID_IHTMLELEMENT7_ONMSTRANSITIONEND, + DISPID_IHTMLELEMENT7_ONMSANIMATIONSTART, + DISPID_IHTMLELEMENT7_ONMSANIMATIONEND, + DISPID_IHTMLELEMENT7_ONMSANIMATIONITERATION, + DISPID_IHTMLELEMENT7_ONMSMANIPULATIONSTATECHANGED, + DISPID_IHTMLELEMENT7_ONINVALID, + DISPID_IHTMLELEMENT7_ONCUECHANGE + }; + unsigned i, a = 0, b = ARRAY_SIZE(attr_dispids) - 1; + + if(get_dispid_type(id) != DISPEXPROP_BUILTIN) + return TRUE; + + while(a < b) { + i = (a + b) / 2; + if(id == attr_dispids[i]) + return TRUE; + if(id <= attr_dispids[i]) b = i; + else a = i + 1; + } + + return FALSE; +} + static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LONG *idx, DISPID start, DISPID *dispid) { DISPID id = start; @@ -7691,6 +7838,8 @@ static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LO return hres; else if(hres == S_FALSE) break; + else if(!is_valid_attr_dispid(id)) + continue; len++; if(len == *idx) @@ -7727,7 +7876,8 @@ static inline HRESULT get_attr_dispid_by_name(HTMLAttributeCollection *This, con } } - return dispex_get_id(&This->elem->node.event_target.dispex, name, fdexNameCaseInsensitive, id); + hres = dispex_get_id(&This->elem->node.event_target.dispex, name, fdexNameCaseInsensitive, id); + return (FAILED(hres) || is_valid_attr_dispid(*id)) ? hres : DISP_E_UNKNOWNNAME; } static inline HRESULT get_domattr(HTMLAttributeCollection *This, DISPID id, LONG *list_pos, HTMLDOMAttribute **attr) diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index 9eb7bac9a11..b67ed183d67 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -5116,6 +5116,24 @@ static void _link_put_href(unsigned line, IHTMLElement *elem, const WCHAR *v) _test_link_href(line, elem, v); } +#define elem_has_attr(a,b) _elem_has_attr(__LINE__,a,b) +static BOOL _elem_has_attr(unsigned line, IUnknown *unk, const WCHAR *attr_name) +{ + BSTR str = SysAllocString(attr_name); + IHTMLElement6 *elem; + VARIANT_BOOL b; + HRESULT hres; + + hres = IUnknown_QueryInterface(unk, &IID_IHTMLElement6, (void**)&elem); + ok_(__FILE__,line)(hres == S_OK, "Could not get IHTMLElement6: %08lx\n", hres); + hres = IHTMLElement6_hasAttribute(elem, str, &b); + ok_(__FILE__,line)(hres == S_OK, "hasAttribute failed: %08lx\n", hres); + IHTMLElement6_Release(elem); + SysFreeString(str); + + return b != VARIANT_FALSE; +} + #define get_elem_doc(e) _get_elem_doc(__LINE__,e) static IHTMLDocument2 *_get_elem_doc(unsigned line, IUnknown *unk) { @@ -10076,8 +10094,31 @@ static void test_attr_node(IHTMLDOMAttribute *test_attr, IHTMLDocument2 *doc) static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) { + static const WCHAR *elem_attr_props[] = { + L"accessKey", L"contentEditable", L"dir", L"disabled", L"hideFocus", L"id", L"lang", L"language", L"onactivate", L"onafterupdate", L"onbeforeactivate", + L"onbeforecopy", L"onbeforecut", L"onbeforedeactivate", L"onbeforeeditfocus", L"onbeforepaste", L"onbeforeupdate", L"onblur", L"oncellchange", L"onclick", + L"oncontextmenu", L"oncontrolselect", L"oncopy", L"oncut", L"ondataavailable", L"ondatasetchanged", L"ondatasetcomplete", L"ondblclick", L"ondeactivate", + L"ondrag", L"ondragend", L"ondragenter", L"ondragleave", L"ondragover", L"ondragstart", L"ondrop", L"onerrorupdate", L"onfilterchange", L"onfocus", + L"onfocusin", L"onfocusout", L"onhelp", L"onkeydown", L"onkeypress", L"onkeyup", L"onlayoutcomplete", L"onlosecapture", L"onmousedown", L"onmouseleave", + L"onmousemove", L"onmouseout", L"onmouseover", L"onmouseup", L"onmousewheel", L"onmove", L"onmoveend", L"onmovestart", L"onpage", L"onpaste", + L"onpropertychange", L"onreadystatechange", L"onresize", L"onresizeend", L"onresizestart", L"onrowenter", L"onrowexit", L"onrowsdelete", + L"onrowsinserted", L"onscroll", L"onselectstart", L"style", L"tabIndex", L"title" + }; + static const WCHAR *elem_noattr_props[] = { + L"addBehavior", L"all", L"applyElement", L"attachEvent", L"attributes", L"behaviorUrns", L"blur", L"canHaveHTML", L"childNodes", L"children", L"className", + L"clearAttributes", L"click", L"clientHeight", L"clientLeft", L"clientTop", L"clientWidth", L"componentFromPoint", L"contains", L"createControlRange", + L"currentStyle", L"detachEvent", L"document", L"doScroll", L"dragDrop", L"filters", L"fireEvent", L"firstChild", L"focus", L"getAdjacentText", L"getAttribute", + L"getAttributeNode", L"getBoundingClientRect", L"getClientRects", L"getElementsByTagName", L"getExpression", L"innerHTML", L"innerText", L"insertAdjacentElement", + L"insertAdjacentHTML", L"insertAdjacentText", L"isContentEditable", L"isDisabled", L"isMultiLine", L"isTextEdit", L"lastChild", L"mergeAttributes", L"nextSibling", + L"nodeName", L"nodeName", L"nodeType", L"nodeValue", L"normalize", L"offsetHeight", L"offsetLeft", L"offsetParent", L"offsetTop", L"offsetWidth", L"outerHTML", + L"outerText", L"ownerDocument", L"parentElement", L"parentNode", L"parentTextEdit", L"previousSibling", L"readyState", L"recordNumber", L"removeAttribute", + L"removeAttributeNode", L"removeBehavior", L"removeExpression", L"replaceAdjacentText", L"runtimeStyle", L"scopeName", L"scrollHeight", L"scrollIntoView", + L"scrollLeft", L"scrollTop", L"scrollWidth", L"setActive", L"setAttribute", L"setAttributeNode", L"setExpression", L"sourceIndex", L"tagName", L"tagUrn", + L"toString", L"uniqueID", L"uniqueNumber" + }; IHTMLDOMAttribute *attr, *attr2, *attr3; IHTMLElement4 *elem4; + unsigned i; VARIANT v; HRESULT hres; @@ -10165,6 +10206,24 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) test_attr_specified(attr, VARIANT_FALSE); test_attr_expando(attr, VARIANT_FALSE); test_attr_node(attr, doc); + + for(i = 0; i < ARRAY_SIZE(elem_attr_props); i++) { + BOOL has_attr = elem_has_attr((IUnknown*)elem, elem_attr_props[i]); + + attr = get_elem_attr_node((IUnknown*)elem, elem_attr_props[i], TRUE); + test_attr_specified(attr, has_attr ? VARIANT_TRUE : VARIANT_FALSE); + test_attr_expando(attr, VARIANT_FALSE); + IHTMLDOMAttribute_Release(attr); + } + for(i = 0; i < ARRAY_SIZE(elem_noattr_props); i++) { + get_elem_attr_node((IUnknown*)elem, elem_noattr_props[i], FALSE); + } + + ok(elem_has_attr((IUnknown*)elem, L"emptyattr"), "elem does not have emptyattr"); + attr = get_elem_attr_node((IUnknown*)elem, L"emptyattr", TRUE); + test_attr_specified(attr, VARIANT_TRUE); + test_attr_expando(attr, VARIANT_TRUE); + test_attr_node(attr, doc); IHTMLDOMAttribute_Release(attr); /* Test created, detached attribute. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 65 +++++++++++++++++----------- dlls/mshtml/tests/dom.c | 95 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 130 insertions(+), 30 deletions(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index 621bc705da3..b3ebfbb4007 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -47,6 +47,8 @@ typedef struct { } tag_desc_t; static HRESULT HTMLElement_Ctor(HTMLDocumentNode*,nsIDOMElement*,HTMLElement**); +static HRESULT get_attr_dispid_by_name(HTMLAttributeCollection*,const WCHAR*,BOOL,DISPID*); +static HRESULT get_domattr(HTMLAttributeCollection*,DISPID,LONG*,HTMLDOMAttribute**); static const tag_desc_t tag_descs[] = { {L"A", HTMLAnchorElement_Create}, @@ -4717,21 +4719,28 @@ static HRESULT WINAPI HTMLElement6_removeAttribute(IHTMLElement6 *iface, BSTR st static HRESULT WINAPI HTMLElement6_getAttributeNode(IHTMLElement6 *iface, BSTR strAttributeName, IHTMLDOMAttribute2 **ppretAttribute) { HTMLElement *This = impl_from_IHTMLElement6(iface); - IHTMLDOMAttribute *attr; + HTMLAttributeCollection *attrs; + HTMLDOMAttribute *attr; HRESULT hres; + DISPID id; - WARN("(%p)->(%s %p) forwarding to IHTMLElement4\n", This, debugstr_w(strAttributeName), ppretAttribute); + TRACE("(%p)->(%s %p)\n", This, debugstr_w(strAttributeName), ppretAttribute); - hres = IHTMLElement4_getAttributeNode(&This->IHTMLElement4_iface, strAttributeName, &attr); + hres = HTMLElement_get_attr_col(&This->node, &attrs); if(FAILED(hres)) return hres; - if(attr) { - hres = IHTMLDOMAttribute_QueryInterface(attr, &IID_IHTMLDOMAttribute2, (void**)ppretAttribute); - IHTMLDOMAttribute_Release(attr); - }else { + hres = get_attr_dispid_by_name(attrs, strAttributeName, TRUE, &id); + if(hres == S_OK) { + hres = get_domattr(attrs, id, NULL, &attr); + if(hres == S_OK) + *ppretAttribute = &attr->IHTMLDOMAttribute2_iface; + }else if(hres == DISP_E_UNKNOWNNAME) { *ppretAttribute = NULL; + hres = S_OK; } + + IHTMLAttributeCollection_Release(&attrs->IHTMLAttributeCollection_iface); return hres; } @@ -7677,7 +7686,7 @@ static HRESULT create_filters_collection(compat_mode_t compat_mode, IHTMLFilters return S_OK; } -static inline BOOL is_valid_attr_dispid(DISPID id) +static inline BOOL is_valid_attr_dispid(HTMLAttributeCollection *col, DISPID id, BOOL specified_only) { /* Builtin props that can be treated as attributes; MUST be sorted by underlying dispid value. */ static const DISPID attr_dispids[] = { @@ -7815,8 +7824,11 @@ static inline BOOL is_valid_attr_dispid(DISPID id) while(a < b) { i = (a + b) / 2; - if(id == attr_dispids[i]) + if(id == attr_dispids[i]) { + if(specified_only) + return !!element_has_attribute(col->elem, dispex_builtin_prop_name(&col->elem->node.event_target.dispex, id)); return TRUE; + } if(id <= attr_dispids[i]) b = i; else a = i + 1; } @@ -7824,7 +7836,8 @@ static inline BOOL is_valid_attr_dispid(DISPID id) return FALSE; } -static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LONG *idx, DISPID start, DISPID *dispid) +static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LONG *idx, DISPID start, + BOOL specified_only, DISPID *dispid) { DISPID id = start; LONG len = -1; @@ -7838,7 +7851,7 @@ static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LO return hres; else if(hres == S_FALSE) break; - else if(!is_valid_attr_dispid(id)) + else if(!is_valid_attr_dispid(This, id, specified_only)) continue; len++; @@ -7855,12 +7868,12 @@ static HRESULT get_attr_dispid_by_relative_idx(HTMLAttributeCollection *This, LO return S_OK; } -static HRESULT get_attr_dispid_by_idx(HTMLAttributeCollection *This, LONG *idx, DISPID *dispid) +static HRESULT get_attr_dispid_by_idx(HTMLAttributeCollection *This, LONG *idx, BOOL specified_only, DISPID *dispid) { - return get_attr_dispid_by_relative_idx(This, idx, DISPID_STARTENUM, dispid); + return get_attr_dispid_by_relative_idx(This, idx, DISPID_STARTENUM, specified_only, dispid); } -static inline HRESULT get_attr_dispid_by_name(HTMLAttributeCollection *This, const WCHAR *name, DISPID *id) +static HRESULT get_attr_dispid_by_name(HTMLAttributeCollection *This, const WCHAR *name, BOOL specified_only, DISPID *id) { HRESULT hres; @@ -7870,17 +7883,17 @@ static inline HRESULT get_attr_dispid_by_name(HTMLAttributeCollection *This, con idx = wcstoul(name, &end_ptr, 10); if(!*end_ptr) { - hres = get_attr_dispid_by_idx(This, &idx, id); + hres = get_attr_dispid_by_idx(This, &idx, specified_only, id); if(SUCCEEDED(hres)) return hres; } } hres = dispex_get_id(&This->elem->node.event_target.dispex, name, fdexNameCaseInsensitive, id); - return (FAILED(hres) || is_valid_attr_dispid(*id)) ? hres : DISP_E_UNKNOWNNAME; + return (FAILED(hres) || is_valid_attr_dispid(This, *id, specified_only)) ? hres : DISP_E_UNKNOWNNAME; } -static inline HRESULT get_domattr(HTMLAttributeCollection *This, DISPID id, LONG *list_pos, HTMLDOMAttribute **attr) +static HRESULT get_domattr(HTMLAttributeCollection *This, DISPID id, LONG *list_pos, HTMLDOMAttribute **attr) { HTMLDOMAttribute *iter; LONG pos = 0; @@ -7978,7 +7991,7 @@ static HRESULT WINAPI HTMLAttributeCollectionEnum_Next(IEnumVARIANT *iface, ULON TRACE("(%p)->(%lu %p %p)\n", This, celt, rgVar, pCeltFetched); for(i = 0; i < celt; i++) { - hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, dispid, &tmp); + hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, dispid, FALSE, &tmp); if(SUCCEEDED(hres)) { dispid = tmp; hres = get_domattr(This->col, dispid, NULL, &attr); @@ -8015,14 +8028,14 @@ static HRESULT WINAPI HTMLAttributeCollectionEnum_Skip(IEnumVARIANT *iface, ULON return S_OK; rel_index = -1; - hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, This->iter_dispid, NULL); + hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, This->iter_dispid, FALSE, NULL); if(FAILED(hres)) return hres; remaining = min(celt, rel_index); if(remaining) { rel_index = remaining - 1; - hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, This->iter_dispid, &dispid); + hres = get_attr_dispid_by_relative_idx(This->col, &rel_index, This->iter_dispid, FALSE, &dispid); if(FAILED(hres)) return hres; This->iter_dispid = dispid; @@ -8074,7 +8087,7 @@ static HRESULT WINAPI HTMLAttributeCollection_get_length(IHTMLAttributeCollectio TRACE("(%p)->(%p)\n", This, p); *p = -1; - hres = get_attr_dispid_by_idx(This, p, NULL); + hres = get_attr_dispid_by_idx(This, p, FALSE, NULL); return hres; } @@ -8111,10 +8124,10 @@ static HRESULT WINAPI HTMLAttributeCollection_item(IHTMLAttributeCollection *ifa switch(V_VT(name)) { case VT_I4: - hres = get_attr_dispid_by_idx(This, &V_I4(name), &id); + hres = get_attr_dispid_by_idx(This, &V_I4(name), FALSE, &id); break; case VT_BSTR: - hres = get_attr_dispid_by_name(This, V_BSTR(name), &id); + hres = get_attr_dispid_by_name(This, V_BSTR(name), FALSE, &id); break; default: FIXME("unsupported name %s\n", debugstr_variant(name)); @@ -8164,7 +8177,7 @@ static HRESULT WINAPI HTMLAttributeCollection2_getNamedItem(IHTMLAttributeCollec TRACE("(%p)->(%s %p)\n", This, debugstr_w(bstrName), newretNode); - hres = get_attr_dispid_by_name(This, bstrName, &id); + hres = get_attr_dispid_by_name(This, bstrName, FALSE, &id); if(hres == DISP_E_UNKNOWNNAME) { *newretNode = NULL; return S_OK; @@ -8249,7 +8262,7 @@ static HRESULT WINAPI HTMLAttributeCollection3_item(IHTMLAttributeCollection3 *i TRACE("(%p)->(%ld %p)\n", This, index, ppNodeOut); - hres = get_attr_dispid_by_idx(This, &index, &id); + hres = get_attr_dispid_by_idx(This, &index, FALSE, &id); if(hres == DISP_E_UNKNOWNNAME) return E_INVALIDARG; if(FAILED(hres)) @@ -8345,7 +8358,7 @@ static HRESULT HTMLAttributeCollection_get_dispid(DispatchEx *dispex, const WCHA TRACE("(%p)->(%s %lx %p)\n", This, debugstr_w(name), flags, dispid); - hres = get_attr_dispid_by_name(This, name, dispid); + hres = get_attr_dispid_by_name(This, name, FALSE, dispid); if(FAILED(hres)) return hres; diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index b67ed183d67..d696902423d 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -2088,10 +2088,10 @@ static void _test_comment_text(unsigned line, IUnknown *unk, const WCHAR *extext } #define create_attr(a,b) _create_attr(__LINE__,a,b) -static IHTMLDOMAttribute *_create_attr(unsigned line, IUnknown *unk, const char *name) +static IHTMLDOMAttribute *_create_attr(unsigned line, IUnknown *unk, const WCHAR *name) { IHTMLDocument5 *doc = _get_htmldoc5_iface(line, unk); - BSTR str = SysAllocString(L"Test"); + BSTR str = SysAllocString(name); IHTMLDOMAttribute *attr; HRESULT hres; @@ -5176,6 +5176,78 @@ static IHTMLDOMAttribute *_get_elem_attr_node(unsigned line, IUnknown *unk, cons return attr; } +#define get_elem6_attr_node(a,b,c) _get_elem6_attr_node(__LINE__,a,b,c) +static IHTMLDOMAttribute *_get_elem6_attr_node(unsigned line, IUnknown *unk, const WCHAR *attr_name, BOOL expect_success) +{ + BSTR str = SysAllocString(attr_name); + IHTMLDOMAttribute2 *attr2; + IHTMLDOMAttribute *attr; + IHTMLElement6 *elem; + HRESULT hres; + + hres = IUnknown_QueryInterface(unk, &IID_IHTMLElement6, (void**)&elem); + ok_(__FILE__,line)(hres == S_OK, "Could not get IHTMLElement6: %08lx\n", hres); + + attr = (void*)0xdeadbeef; + hres = IHTMLElement6_getAttributeNode(elem, str, &attr2); + ok_(__FILE__,line)(hres == S_OK, "getAttributeNode failed: %08lx\n", hres); + if(expect_success) { + ok_(__FILE__,line)(attr2 != NULL, "attr = NULL\n"); + hres = IHTMLDOMAttribute2_QueryInterface(attr2, &IID_IHTMLDOMAttribute, (void**)&attr); + ok_(__FILE__,line)(hres == S_OK, "Could not get IHTMLDOMAttribute: %08lx\n", hres); + IHTMLDOMAttribute2_Release(attr2); + }else { + ok_(__FILE__,line)(!attr2, "attr = %p\n", attr2); + attr = NULL; + } + + IHTMLElement6_Release(elem); + SysFreeString(str); + return attr; +} + +#define get_elem_attr_node_via_disp(a,b,c) _get_elem_attr_node_via_disp(__LINE__,a,b,c) +static IHTMLDOMAttribute *_get_elem_attr_node_via_disp(unsigned line, IUnknown *unk, const WCHAR *attr_name, BOOL expect_success) +{ + IHTMLDOMAttribute *attr = NULL; + DISPPARAMS dp = { 0 }; + IDispatchEx *dispex; + VARIANT var, arg; + EXCEPINFO ei; + HRESULT hres; + DISPID id; + BSTR str; + + hres = IUnknown_QueryInterface(unk, &IID_IDispatchEx, (void**)&dispex); + ok_(__FILE__,line)(hres == S_OK, "Could not get IDispatchEx: %08lx\n", hres); + + str = SysAllocString(L"getAttributeNode"); + hres = IDispatchEx_GetDispID(dispex, str, fdexNameCaseSensitive, &id); + ok_(__FILE__,line)(hres == S_OK, "GetDispID failed: %08lx\n", hres); + SysFreeString(str); + + dp.cArgs = 1; + dp.rgvarg = &arg; + V_VT(&arg) = VT_BSTR; + V_BSTR(&arg) = SysAllocString(attr_name); + + VariantInit(&var); + hres = IDispatchEx_InvokeEx(dispex, id, LOCALE_NEUTRAL, DISPATCH_METHOD, &dp, &var, &ei, NULL); + ok_(__FILE__,line)(hres == S_OK, "InvokeEx failed: %08lx\n", hres); + if(!expect_success) + ok_(__FILE__,line)(V_VT(&var) == VT_NULL, "V_VT(var) = %d\n", V_VT(&var)); + else { + ok_(__FILE__,line)(V_VT(&var) == VT_DISPATCH, "V_VT(var) = %d\n", V_VT(&var)); + ok_(__FILE__,line)(V_DISPATCH(&var) != NULL, "V_DISPATCH(var) == NULL\n"); + hres = IDispatch_QueryInterface(V_DISPATCH(&var), &IID_IHTMLDOMAttribute, (void**)&attr); + ok_(__FILE__,line)(hres == S_OK, "Could not get IHTMLDOMAttribute: %08lx\n", hres); + } + IDispatchEx_Release(dispex); + VariantClear(&arg); + VariantClear(&var); + return attr; +} + #define get_attr_node_value(a,b,c) _get_attr_node_value(__LINE__,a,b,c) static void _get_attr_node_value(unsigned line, IHTMLDOMAttribute *attr, VARIANT *v, VARTYPE vt) { @@ -10214,9 +10286,14 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) test_attr_specified(attr, has_attr ? VARIANT_TRUE : VARIANT_FALSE); test_attr_expando(attr, VARIANT_FALSE); IHTMLDOMAttribute_Release(attr); + attr = get_elem6_attr_node((IUnknown*)elem, elem_attr_props[i], has_attr); + if(attr) IHTMLDOMAttribute_Release(attr); + attr = get_elem_attr_node_via_disp((IUnknown*)elem, elem_attr_props[i], TRUE); + if(attr) IHTMLDOMAttribute_Release(attr); } for(i = 0; i < ARRAY_SIZE(elem_noattr_props); i++) { get_elem_attr_node((IUnknown*)elem, elem_noattr_props[i], FALSE); + get_elem6_attr_node((IUnknown*)elem, elem_noattr_props[i], FALSE); } ok(elem_has_attr((IUnknown*)elem, L"emptyattr"), "elem does not have emptyattr"); @@ -10226,8 +10303,18 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) test_attr_node(attr, doc); IHTMLDOMAttribute_Release(attr); + attr = get_elem6_attr_node((IUnknown*)elem, L"emptyattr", TRUE); + test_attr_specified(attr, VARIANT_TRUE); + test_attr_expando(attr, VARIANT_TRUE); + IHTMLDOMAttribute_Release(attr); + + attr = get_elem_attr_node_via_disp((IUnknown*)elem, L"emptyattr", TRUE); + test_attr_specified(attr, VARIANT_TRUE); + test_attr_expando(attr, VARIANT_TRUE); + IHTMLDOMAttribute_Release(attr); + /* Test created, detached attribute. */ - attr = create_attr((IUnknown*)doc, "Test"); + attr = create_attr((IUnknown*)doc, L"Test"); test_disp((IUnknown*)attr, &DIID_DispHTMLDOMAttribute, NULL, L"[object]"); test_ifaces((IUnknown*)attr, attr_iids); @@ -10270,7 +10357,7 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) ok(iface_cmp((IUnknown*)attr2, (IUnknown*)attr), "attr2 != attr\n"); IHTMLDOMAttribute_Release(attr2); - attr3 = create_attr((IUnknown*)doc, "Test"); + attr3 = create_attr((IUnknown*)doc, L"Test"); put_attr_value(attr3, L"replace test"); hres = IHTMLElement4_setAttributeNode(elem4, attr3, &attr2); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlattr.c | 6 +++--- dlls/mshtml/mshtml_private.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/htmlattr.c b/dlls/mshtml/htmlattr.c index ff6173aacdb..d1cf1742382 100644 --- a/dlls/mshtml/htmlattr.c +++ b/dlls/mshtml/htmlattr.c @@ -51,7 +51,7 @@ static HRESULT WINAPI HTMLDOMAttribute_get_nodeName(IHTMLDOMAttribute *iface, BS return E_FAIL; } - *p = SysAllocString(This->name); + *p = SysAllocStringLen(This->name, SysStringLen(This->name)); return *p ? S_OK : E_OUTOFMEMORY; } @@ -528,7 +528,7 @@ static void HTMLDOMAttribute_destructor(DispatchEx *dispex) { HTMLDOMAttribute *This = impl_from_DispatchEx(dispex); VariantClear(&This->value); - free(This->name); + SysFreeString(This->name); free(This); } @@ -600,7 +600,7 @@ HRESULT HTMLDOMAttribute_Create(const WCHAR *name, HTMLElement *elem, DISPID dis /* For detached attributes we may still do most operations if we have its name available. */ if(name) { - ret->name = wcsdup(name); + ret->name = SysAllocString(name); if(!ret->name) { IHTMLDOMAttribute_Release(&ret->IHTMLDOMAttribute_iface); return E_OUTOFMEMORY; diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 4319c8b4725..ca49312fbe3 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1341,7 +1341,7 @@ typedef struct { /* value is valid only for detached attributes (when elem == NULL). */ VARIANT value; /* name must be valid for detached attributes */ - WCHAR *name; + BSTR name; HTMLDocumentNode *doc; HTMLElement *elem; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 43 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index b3ebfbb4007..48543d62480 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -527,6 +527,24 @@ HRESULT create_element(HTMLDocumentNode *doc, const WCHAR *tag, HTMLElement **re return hres; } +static HTMLDOMAttribute *find_attr_in_list(HTMLAttributeCollection *attrs, DISPID dispid, LONG *pos) +{ + HTMLDOMAttribute *iter, *ret = NULL; + struct list *list = &attrs->attrs; + unsigned i = 0; + + LIST_FOR_EACH_ENTRY(iter, list, HTMLDOMAttribute, entry) { + if(iter->dispid == dispid) { + ret = iter; + break; + } + i++; + } + if(pos) + *pos = i; + return ret; +} + typedef struct { DispatchEx dispex; IHTMLRect IHTMLRect_iface; @@ -4392,7 +4410,7 @@ static HRESULT WINAPI HTMLElement4_setAttributeNode(IHTMLElement4 *iface, IHTMLD IHTMLDOMAttribute **ppretAttribute) { HTMLElement *This = impl_from_IHTMLElement4(iface); - HTMLDOMAttribute *attr, *iter, *replace = NULL; + HTMLDOMAttribute *attr, *replace; HTMLAttributeCollection *attrs; DISPID dispid; HRESULT hres; @@ -4416,13 +4434,7 @@ static HRESULT WINAPI HTMLElement4_setAttributeNode(IHTMLElement4 *iface, IHTMLD if(FAILED(hres)) return hres; - LIST_FOR_EACH_ENTRY(iter, &attrs->attrs, HTMLDOMAttribute, entry) { - if(iter->dispid == dispid) { - replace = iter; - break; - } - } - + replace = find_attr_in_list(attrs, dispid, NULL); if(replace) { hres = get_elem_attr_value_by_dispid(This, dispid, &replace->value); if(FAILED(hres)) { @@ -7895,28 +7907,15 @@ static HRESULT get_attr_dispid_by_name(HTMLAttributeCollection *This, const WCHA static HRESULT get_domattr(HTMLAttributeCollection *This, DISPID id, LONG *list_pos, HTMLDOMAttribute **attr) { - HTMLDOMAttribute *iter; - LONG pos = 0; HRESULT hres; - *attr = NULL; - LIST_FOR_EACH_ENTRY(iter, &This->attrs, HTMLDOMAttribute, entry) { - if(iter->dispid == id) { - *attr = iter; - break; - } - pos++; - } - - if(!*attr) { + if(!(*attr = find_attr_in_list(This, id, list_pos))) { hres = HTMLDOMAttribute_Create(NULL, This->elem, id, This->elem->node.doc, attr); if(FAILED(hres)) return hres; } IHTMLDOMAttribute_AddRef(&(*attr)->IHTMLDOMAttribute_iface); - if(list_pos) - *list_pos = pos; return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 25 ++++++++++ dlls/mshtml/tests/dom.c | 107 +++++++++++++++++++++++++++++++++++----- 2 files changed, 120 insertions(+), 12 deletions(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index 48543d62480..0d4474413f8 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -1333,6 +1333,9 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA TRACE("(%p)->(%s %lx %p)\n", This, debugstr_w(strAttributeName), lFlags, pfSuccess); if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) { + HTMLAttributeCollection *attrs; + HTMLDOMAttribute *attr; + hres = dispex_get_id(&This->node.event_target.dispex, translate_attr_name(strAttributeName, compat_mode), lFlags & ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &id); if(hres == DISP_E_UNKNOWNNAME) { @@ -1342,6 +1345,26 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA if(FAILED(hres)) return hres; + hres = HTMLElement_get_attr_col(&This->node, &attrs); + if(FAILED(hres)) + return hres; + attr = find_attr_in_list(attrs, id, NULL); + IHTMLAttributeCollection_Release(&attrs->IHTMLAttributeCollection_iface); + + if(attr) { + hres = get_elem_attr_value_by_dispid(This, id, &attr->value); + if(FAILED(hres)) + return hres; + if(!attr->name) { + hres = dispex_prop_name(&This->node.event_target.dispex, id, &attr->name); + if(FAILED(hres)) + return hres; + } + attr->elem = NULL; + list_remove(&attr->entry); + IHTMLDOMNode_Release(&This->node.IHTMLDOMNode_iface); + } + if(id == DISPID_IHTMLELEMENT_STYLE) { IHTMLStyle *style; @@ -8366,6 +8389,8 @@ static HRESULT HTMLAttributeCollection_get_dispid(DispatchEx *dispex, const WCHA return hres; IHTMLDOMAttribute_Release(&attr->IHTMLDOMAttribute_iface); + /* Even though this breaks DISPID rules where the same name must return the same DISPID, because the pos can change + * as attributes are removed (and re-added), it's how native works (see test_attr_collection_disp in tests). */ *dispid = MSHTML_DISPID_CUSTOM_MIN+pos; return S_OK; } diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index d696902423d..ff3c64b0515 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -3745,39 +3745,42 @@ static LONG test_attr_collection_attr(IDispatch *attr, LONG i) return ret; } -static void test_attr_collection_disp(IDispatch *disp) +static void test_attr_collection_disp(IDispatch *disp, LONG len, IHTMLElement *elem) { + DISPID dispid, dispid2, dispid_attr1, dispid_attr2, dispid_attr3; IDispatchEx *dispex; IHTMLDOMAttribute *attr; DISPPARAMS dp = {NULL, NULL, 0, 0}; + VARIANT_BOOL vbool; + WCHAR buf[11]; VARIANT var; EXCEPINFO ei; - DISPID id; - BSTR bstr; HRESULT hres; + BSTR bstr; hres = IDispatch_QueryInterface(disp, &IID_IDispatchEx, (void**)&dispex); ok(hres == S_OK, "QueryInterface failed: %08lx\n", hres); - bstr = SysAllocString(L"0"); - hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &id); + swprintf(buf, ARRAY_SIZE(buf), L"%lu", len - 2); + bstr = SysAllocString(buf); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid); ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); SysFreeString(bstr); VariantInit(&var); - hres = IDispatchEx_InvokeEx(dispex, id, LOCALE_NEUTRAL, INVOKE_PROPERTYGET, &dp, &var, &ei, NULL); + hres = IDispatchEx_InvokeEx(dispex, dispid, LOCALE_NEUTRAL, INVOKE_PROPERTYGET, &dp, &var, &ei, NULL); ok(hres == S_OK, "InvokeEx failed: %08lx\n", hres); ok(V_VT(&var) == VT_DISPATCH, "V_VT(var)=%d\n", V_VT(&var)); ok(V_DISPATCH(&var) != NULL, "V_DISPATCH(var) == NULL\n"); VariantClear(&var); bstr = SysAllocString(L"attr1"); - hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &id); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid_attr1); ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); SysFreeString(bstr); VariantInit(&var); - hres = IDispatchEx_InvokeEx(dispex, id, LOCALE_NEUTRAL, INVOKE_PROPERTYGET, &dp, &var, &ei, NULL); + hres = IDispatchEx_InvokeEx(dispex, dispid_attr1, LOCALE_NEUTRAL, INVOKE_PROPERTYGET, &dp, &var, &ei, NULL); ok(hres == S_OK, "InvokeEx failed: %08lx\n", hres); ok(V_VT(&var) == VT_DISPATCH, "V_VT(var)=%d\n", V_VT(&var)); ok(V_DISPATCH(&var) != NULL, "V_DISPATCH(var) == NULL\n"); @@ -3789,6 +3792,72 @@ static void test_attr_collection_disp(IDispatch *disp) IHTMLDOMAttribute_Release(attr); VariantClear(&var); + bstr = SysAllocString(L"attr2"); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid_attr2); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + SysFreeString(bstr); + + bstr = SysAllocString(L"attr3"); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid_attr3); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + SysFreeString(bstr); + + ok(dispid == dispid_attr1 || dispid == dispid_attr2 || dispid == dispid_attr3, + "indexed dispid isn't one of the attr dispids\n"); + + bstr = SysAllocString(L"attr1"); + hres = IHTMLElement_removeAttribute(elem, bstr, 0, &vbool); + ok(hres == S_OK, "removeAttribute failed: %08lx\n", hres); + ok(vbool == VARIANT_TRUE, "removeAttribute returned %x\n", vbool); + SysFreeString(bstr); + + hres = IDispatchEx_GetMemberName(dispex, dispid_attr1, &bstr); + ok(hres == S_OK, "GetMemberName failed: %08lx\n", hres); + ok(wcscmp(bstr, L"attr1"), "GetMemberName still returned attr1 after removal\n"); + SysFreeString(bstr); + + bstr = SysAllocString(buf); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid2); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + ok(dispid, "indexed dispid is not the same after removal\n"); + SysFreeString(bstr); + + bstr = SysAllocString(L"attr2"); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + ok(dispid == dispid_attr2, "dispid != dispid_attr2\n"); + SysFreeString(bstr); + + bstr = SysAllocString(L"attr1"); + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid); + ok(hres == DISP_E_UNKNOWNNAME, "GetDispID returned: %08lx\n", hres); + SysFreeString(bstr); + + bstr = SysAllocString(L"added_attr"); + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = SysAllocString(L"test"); + hres = IHTMLElement_setAttribute(elem, bstr, var, 0); + ok(hres == S_OK, "setAttribute failed: %08lx\n", hres); + + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + ok(dispid != dispid_attr1, "added attr dispid == dispid_attr1\n"); + ok(dispid != dispid_attr2, "added attr dispid == dispid_attr2\n"); + ok(dispid != dispid_attr3, "added attr dispid == dispid_attr3\n"); + SysFreeString(bstr); + + bstr = SysAllocString(L"attr1"); + hres = IHTMLElement_setAttribute(elem, bstr, var, 0); + ok(hres == S_OK, "setAttribute failed: %08lx\n", hres); + VariantClear(&var); + + hres = IDispatchEx_GetDispID(dispex, bstr, fdexNameCaseSensitive, &dispid); + ok(hres == S_OK, "GetDispID failed: %08lx\n", hres); + ok(dispid != dispid_attr1, "attr1 after re-added dispid == dispid_attr1\n"); + ok(dispid != dispid_attr2, "attr1 after re-added dispid == dispid_attr2\n"); + ok(dispid != dispid_attr3, "attr1 after re-added dispid == dispid_attr3\n"); + SysFreeString(bstr); + IDispatchEx_Release(dispex); } @@ -3886,7 +3955,7 @@ static void test_attr_collection(IHTMLElement *elem) ok(hres == E_INVALIDARG, "item failed: %08lx\n", hres); VariantClear(&id); - test_attr_collection_disp(disp); + test_attr_collection_disp(disp, len, elem); IDispatch_Release(disp); IHTMLAttributeCollection_Release(attr_col); @@ -10190,9 +10259,11 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) }; IHTMLDOMAttribute *attr, *attr2, *attr3; IHTMLElement4 *elem4; + VARIANT_BOOL vbool; + HRESULT hres; unsigned i; VARIANT v; - HRESULT hres; + BSTR bstr; get_elem_attr_node((IUnknown*)elem, L"noattr", FALSE); @@ -10306,6 +10377,20 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) attr = get_elem6_attr_node((IUnknown*)elem, L"emptyattr", TRUE); test_attr_specified(attr, VARIANT_TRUE); test_attr_expando(attr, VARIANT_TRUE); + + bstr = SysAllocString(L"emptyattr"); + hres = IHTMLElement_removeAttribute(elem, bstr, 0, &vbool); + ok(hres == S_OK, "removeAttribute failed: %08lx\n", hres); + ok(vbool == VARIANT_TRUE, "removeAttribute returned %x\n", vbool); + test_attr_expando(attr, VARIANT_FALSE); + SysFreeString(bstr); + + elem4 = get_elem4_iface((IUnknown*)elem); + hres = IHTMLElement4_setAttributeNode(elem4, attr, &attr2); + ok(hres == S_OK, "setAttributeNode failed: %08lx\n", hres); + ok(!attr2, "attr2 != NULL\n"); + test_attr_specified(attr, VARIANT_TRUE); + test_attr_expando(attr, VARIANT_TRUE); IHTMLDOMAttribute_Release(attr); attr = get_elem_attr_node_via_disp((IUnknown*)elem, L"emptyattr", TRUE); @@ -10343,8 +10428,6 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) SysFreeString(V_BSTR(&v)); test_attr_value(attr, L"testing"); - elem4 = get_elem4_iface((IUnknown*)elem); - hres = IHTMLElement4_setAttributeNode(elem4, attr, &attr2); ok(hres == S_OK, "setAttributeNode failed: %08lx\n", hres); ok(!attr2, "attr2 != NULL\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlattr.c | 4 ++-- dlls/mshtml/tests/dom.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/htmlattr.c b/dlls/mshtml/htmlattr.c index d1cf1742382..8307ab8f3f1 100644 --- a/dlls/mshtml/htmlattr.c +++ b/dlls/mshtml/htmlattr.c @@ -94,8 +94,8 @@ static HRESULT WINAPI HTMLDOMAttribute_get_specified(IHTMLDOMAttribute *iface, V TRACE("(%p)->(%p)\n", This, p); if(!This->elem || !This->elem->dom_element) { - FIXME("NULL This->elem\n"); - return E_UNEXPECTED; + *p = VARIANT_FALSE; + return S_OK; } if(get_dispid_type(This->dispid) != DISPEXPROP_BUILTIN) { diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index ff3c64b0515..d9178c0abe0 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -10382,6 +10382,7 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) hres = IHTMLElement_removeAttribute(elem, bstr, 0, &vbool); ok(hres == S_OK, "removeAttribute failed: %08lx\n", hres); ok(vbool == VARIANT_TRUE, "removeAttribute returned %x\n", vbool); + test_attr_specified(attr, VARIANT_FALSE); test_attr_expando(attr, VARIANT_FALSE); SysFreeString(bstr); @@ -10406,6 +10407,7 @@ static void test_attr(IHTMLDocument2 *doc, IHTMLElement *elem) test_no_iface((IUnknown*)attr, &IID_IHTMLDOMNode); test_attr_node_name(attr, L"Test"); + test_attr_specified(attr, VARIANT_FALSE); test_attr_expando(attr, VARIANT_FALSE); get_attr_node_value(attr, &v, VT_EMPTY); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8251
Jacek Caban (@jacek) commented about dlls/mshtml/htmlelem.c:
return S_OK; }
+static inline BOOL is_valid_attr_dispid(DISPID id) +{ + /* Builtin props that can be treated as attributes; MUST be sorted by underlying dispid value. */
What about attributes from other interfaces? Also, what application depends on all this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_105904
Surprisingly, this applies even in IE9+ modes when using IHTMLElement4's methods, but we can't test that yet because it's too broken in wine.
Frankly, I’d be tempted to ignore such quirks in IE9+, and stick to the standard, unless we find an application that relies on them. I’m worried this could easily become messy... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_105905
Do you mean other interfaces on elements? And well not this patch specifically, but I'm trying to implement them right and pass the tests. Note that attribute collections even in IE9+ expose these unspecified builtin attributes even if they're not "specified" on the element and that's the more important thing than IHTMLElement4, this is just a prequisite to simplify it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_105931
On Mon Jun 9 20:51:10 2025 +0000, Jacek Caban wrote:
Surprisingly, this applies even in IE9+ modes when using IHTMLElement4's methods, but we can't test that yet because it's too broken in wine. Frankly, I’d be tempted to ignore such quirks in IE9+, and stick to the standard, unless we find an application that relies on them. I’m worried this could easily become messy... With regards to this function specifically, it just meant we added the IE9-only DISPIDs (those that don't exist in older modes) to the list, I don't think that's such a big deal?
I only mentioned it because those aren't tested yet since IE9+ attribute nodes are too broken on Wine right now to attempt that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_105932
Do you mean other interfaces on elements?
Things like `IHTMLBodyElement`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_105979
On Mon Jun 9 20:51:10 2025 +0000, Gabriel Ivăncescu wrote:
With regards to this function specifically, it just meant we added the IE9-only DISPIDs (those that don't exist in older modes) to the list, I don't think that's such a big deal? I only mentioned it because those aren't tested yet since IE9+ attribute nodes are too broken on Wine right now to attempt that. I can't tell for sure without seeing the code, but I'm afraid that for IE9+ this can easily get messy. For example, we need Gecko node for our node implementation, unless we'd some ugly things like handling NULL `nsnode` in `HTMLDOMNode` or creating those nodes dynamically.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_106023
On Tue Jun 10 11:03:18 2025 +0000, Jacek Caban wrote:
I can't tell for sure without seeing the code, but I'm afraid that for IE9+ this can easily get messy. For example, we need Gecko node for our node implementation, unless we'd some ugly things like handling NULL `nsnode` in `HTMLDOMNode` or creating those nodes dynamically. Is NULL nsnode really that ugly? The "mode" is tied to the attribute node (I've tested this and have tests for it), even if it's attached to an element / collection that's in another mode (e.g. across iframes). So it's initialized at creation as usual, or set to NULL (also at creation) for legacy nodes.
I mean it still won't be super straightforward of course, since the nodes can also be mixed in a collection, and I don't like the hack Proton uses which is simpler but very wrong. But due to this I'm using the existing list infrastructure for the collection's dispids instead of a completely different one for gecko nodes. Anyway I still need to split that somehow (kinda hard) and now testing the other interfaces for this MR, maybe I'll resort to doing a blacklist instead, since it's safer wrt regressions (and maybe testing with collections is easier since it enumerates all attributes including "unspecified" ones). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8251#note_106077
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)