[PATCH v5 1/2] mshtml: Implement inline attributes parsing for createElement.
We let gecko parse a dummy unknown tag (after validation) using a contextual fragment, and then copy its attributes to the proper element with correct tag. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 134 +++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 63 ++++++++++++++ 2 files changed, 196 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index de69e2f..91ccfb9 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -355,6 +355,134 @@ static inline HTMLElement *impl_from_IHTMLElement(IHTMLElement *iface) return CONTAINING_RECORD(iface, HTMLElement, IHTMLElement_iface); } +static HRESULT copy_nselem_attrs(nsIDOMElement *nselem_with_attrs, nsIDOMElement *nselem) +{ + nsIDOMMozNamedAttrMap *attrs; + nsAString name_str, val_str; + nsresult nsres, nsres2; + nsIDOMAttr *attr; + UINT32 i, length; + + nsres = nsIDOMElement_GetAttributes(nselem_with_attrs, &attrs); + if(NS_FAILED(nsres)) + return E_FAIL; + + nsres = nsIDOMMozNamedAttrMap_GetLength(attrs, &length); + if(NS_FAILED(nsres)) { + nsIDOMMozNamedAttrMap_Release(attrs); + return E_FAIL; + } + + nsAString_Init(&name_str, NULL); + nsAString_Init(&val_str, NULL); + for(i = 0; i < length; i++) { + nsres = nsIDOMMozNamedAttrMap_Item(attrs, i, &attr); + if(NS_FAILED(nsres)) + continue; + + nsres = nsIDOMAttr_GetNodeName(attr, &name_str); + nsres2 = nsIDOMAttr_GetNodeValue(attr, &val_str); + nsIDOMAttr_Release(attr); + if(NS_FAILED(nsres) || NS_FAILED(nsres2)) + continue; + + nsIDOMElement_SetAttribute(nselem, &name_str, &val_str); + } + nsAString_Finish(&name_str); + nsAString_Finish(&val_str); + + nsIDOMMozNamedAttrMap_Release(attrs); + return S_OK; +} + +static HRESULT create_nselem_parse(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) +{ + static const WCHAR prefix[4] = L"<FOO"; + nsIDOMDocumentFragment *nsfragment; + WCHAR *p = wcschr(tag + 1, '>'); + UINT32 i, name_len, size; + nsIDOMElement *nselem; + nsIDOMRange *nsrange; + nsIDOMNode *nsnode; + nsresult nsres; + nsAString str; + HRESULT hres; + + if(!p || p[1] || wcschr(tag + 1, '<')) + return E_FAIL; + if(!doc->nsdoc) { + WARN("NULL nsdoc\n"); + return E_UNEXPECTED; + } + + /* Ignore the starting token and > or /> end token */ + name_len = p - tag - 1 - (p[-1] == '/'); + + /* Get the tag name using HTML whitespace rules */ + for(i = 1; i <= name_len; i++) { + if((tag[i] >= 0x09 && tag[i] <= 0x0d) || tag[i] == ' ') { + name_len = i - 1; + break; + } + } + if(!name_len) + return E_FAIL; + size = (p + 2 - (tag + 1 + name_len)) * sizeof(WCHAR); + + /* Parse the input via a contextual fragment, using a dummy unknown tag */ + nsres = nsIDOMHTMLDocument_CreateRange(doc->nsdoc, &nsrange); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + if(!(p = heap_alloc(sizeof(prefix) + size))) { + nsIDOMRange_Release(nsrange); + return E_OUTOFMEMORY; + } + memcpy(p, prefix, sizeof(prefix)); + memcpy(p + ARRAY_SIZE(prefix), tag + 1 + name_len, size); + + nsAString_InitDepend(&str, p); + nsIDOMRange_CreateContextualFragment(nsrange, &str, &nsfragment); + nsIDOMRange_Release(nsrange); + nsAString_Finish(&str); + heap_free(p); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + /* Grab the parsed element and copy its attributes into the proper element */ + nsres = nsIDOMDocumentFragment_GetFirstChild(nsfragment, &nsnode); + nsIDOMDocumentFragment_Release(nsfragment); + if(NS_FAILED(nsres) || !nsnode) + return E_FAIL; + + nsres = nsIDOMNode_QueryInterface(nsnode, &IID_nsIDOMElement, (void**)&nselem); + nsIDOMNode_Release(nsnode); + if(NS_FAILED(nsres)) + return E_FAIL; + + if(!(p = heap_alloc((name_len + 1) * sizeof(WCHAR)))) + hres = E_OUTOFMEMORY; + else { + memcpy(p, tag + 1, name_len * sizeof(WCHAR)); + p[name_len] = '\0'; + + nsAString_InitDepend(&str, p); + nsres = nsIDOMHTMLDocument_CreateElement(doc->nsdoc, &str, ret); + nsAString_Finish(&str); + heap_free(p); + + if(NS_FAILED(nsres)) + hres = map_nsresult(nsres); + else { + hres = copy_nselem_attrs(nselem, *ret); + if(FAILED(hres)) + nsIDOMElement_Release(*ret); + } + } + nsIDOMElement_Release(nselem); + return hres; +} + HRESULT create_nselem(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) { nsAString tag_str; @@ -385,7 +513,11 @@ HRESULT create_element(HTMLDocumentNode *doc, const WCHAR *tag, HTMLElement **re if(!doc->nsdoc) doc = doc->node.doc; - hres = create_nselem(doc, tag, &nselem); + /* IE8 and below allow creating elements with attributes, such as <div class="a"> */ + if(tag[0] == '<' && dispex_compat_mode(&doc->node.event_target.dispex) <= COMPAT_MODE_IE8) + hres = create_nselem_parse(doc, tag, &nselem); + else + hres = create_nselem(doc, tag, &nselem); if(FAILED(hres)) return hres; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 02a5f5f..d566223 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -488,6 +488,69 @@ sync_test("style_props", function() { } }); +sync_test("createElement_inline_attr", function() { + var v = document.documentMode, e, s; + + if(v < 9) { + s = document.createElement("<div>").tagName; + ok(s === "DIV", "<div>.tagName returned " + s); + s = document.createElement("<div >").tagName; + ok(s === "DIV", "<div >.tagName returned " + s); + s = document.createElement("<div/>").tagName; + ok(s === "DIV", "<div/>.tagName returned " + s); + e = 0; + try { + document.createElement("<div"); + }catch(ex) { + e = ex.number; + } + ok(e === 0x4005 - 0x80000000, "<div e = " + e); + e = 0; + try { + document.createElement("<div test=1"); + }catch(ex) { + e = ex.number; + } + ok(e === 0x4005 - 0x80000000, "<div test=1 e = " + e); + + var tags = [ "div", "head", "body", "title", "html" ]; + + for(var i = 0; i < tags.length; i++) { + e = document.createElement("<" + tags[i] + " test='a\"' abcd=\""b"\">"); + ok(e.tagName === tags[i].toUpperCase(), "<" + tags[i] + " test=\"a\" abcd=\"b\">.tagName returned " + e.tagName); + todo_wine_if(v == 8). + ok(e.test === "a\"", "<" + tags[i] + " test='a\"' abcd=\""b"\">.test returned " + e.test); + todo_wine_if(v == 8). + ok(e.abcd === "\"b\"", "<" + tags[i] + " test='a\"' abcd=\""b"\">.abcd returned " + e.abcd); + } + }else { + s = ""; + e = 0; + try { + document.createElement("<div>"); + }catch(ex) { + s = ex.toString(); + e = ex.number; + } + todo_wine. + ok(e === undefined, "<div> e = " + e); + todo_wine. + ok(s === "InvalidCharacterError", "<div> s = " + s); + s = ""; + e = 0; + try { + document.createElement("<div test=\"a\">"); + }catch(ex) { + s = ex.toString(); + e = ex.number; + } + todo_wine. + ok(e === undefined, "<div test=\"a\"> e = " + e); + todo_wine. + ok(s === "InvalidCharacterError", "<div test=\"a\"> s = " + s); + } +}); + sync_test("JS objs", function() { var g = window; -- 2.31.1
For IE modes 8 and above, IHTMLElement::setAttribute uses the underlying nsIDOMElement::SetAttribute, which does nothing since we're already enumerating the ns attributes, and we're supposed to create the DISPIDs for them. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 5 ++++- dlls/mshtml/tests/documentmode.js | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index 91ccfb9..9133a46 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -6516,7 +6516,10 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex) } else V_BSTR(&value) = NULL; - IHTMLElement_setAttribute(&This->IHTMLElement_iface, name, value, 0); + hres = IDispatchEx_GetDispID(&dispex->IDispatchEx_iface, name, fdexNameEnsure | fdexNameCaseInsensitive, &id); + if(SUCCEEDED(hres)) + set_elem_attr_value_by_dispid(This, id, &value); + SysFreeString(name); VariantClear(&value); } diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index d566223..e7a3829 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -518,9 +518,7 @@ sync_test("createElement_inline_attr", function() { for(var i = 0; i < tags.length; i++) { e = document.createElement("<" + tags[i] + " test='a\"' abcd=\""b"\">"); ok(e.tagName === tags[i].toUpperCase(), "<" + tags[i] + " test=\"a\" abcd=\"b\">.tagName returned " + e.tagName); - todo_wine_if(v == 8). ok(e.test === "a\"", "<" + tags[i] + " test='a\"' abcd=\""b"\">.test returned " + e.test); - todo_wine_if(v == 8). ok(e.abcd === "\"b\"", "<" + tags[i] + " test='a\"' abcd=\""b"\">.abcd returned " + e.abcd); } }else { -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=101582 Your paranoid android. === w8adm (32 bit report) === mshtml: events.c:1089: Test failed: unexpected call img_onerror events: Timeout === w8 (32 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w8adm (32 bit report) === mshtml: htmldoc.c:3084: Test failed: Incorrect error code: -2146697211 htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:5861: Test failed: expected OnChanged_1012 htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV htmldoc.c:5864: Test failed: expected Exec_SETTITLE htmldoc.c:5905: Test failed: expected FireNavigateComplete2 === w7u_adm (32 bit report) === mshtml: script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1636498770036 expected 1636498770099"
Hi Gabriel, On 11/9/21 10:00 PM, Gabriel Ivăncescu wrote:
--- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -6516,7 +6516,10 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex) } else V_BSTR(&value) = NULL;
- IHTMLElement_setAttribute(&This->IHTMLElement_iface, name, value, 0); + hres = IDispatchEx_GetDispID(&dispex->IDispatchEx_iface, name, fdexNameEnsure | fdexNameCaseInsensitive, &id); + if(SUCCEEDED(hres)) + set_elem_attr_value_by_dispid(This, id, &value);
While this is probably the right thing for compat modes <IE9, later modes should not really need it. They should not expose attributes as JS properties, see the attached test. I think that entire HTMLElement_populate_props should be no-op on IE9+. That means that current attributes collection will not work for those cases, but AFICS it's already broken. The right fix for attributes collection would be to have its variant based on something like nsIDOMMozNamedAttrMap. Thanks, Jacek
On 10/11/2021 17:29, Jacek Caban wrote:
Hi Gabriel,
On 11/9/21 10:00 PM, Gabriel Ivăncescu wrote:
--- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -6516,7 +6516,10 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex) } else V_BSTR(&value) = NULL; - IHTMLElement_setAttribute(&This->IHTMLElement_iface, name, value, 0); + hres = IDispatchEx_GetDispID(&dispex->IDispatchEx_iface, name, fdexNameEnsure | fdexNameCaseInsensitive, &id); + if(SUCCEEDED(hres)) + set_elem_attr_value_by_dispid(This, id, &value);
While this is probably the right thing for compat modes <IE9, later modes should not really need it. They should not expose attributes as JS properties, see the attached test. I think that entire HTMLElement_populate_props should be no-op on IE9+. That means that current attributes collection will not work for those cases, but AFICS it's already broken. The right fix for attributes collection would be to have its variant based on something like nsIDOMMozNamedAttrMap.
Thanks,
Jacek
Ah, thanks for noticing, I'll just exit early on IE9+ then. This gives me more reasons to attempt to fix it at some point since already had the issue with toString exposed as an attribute...
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban -
Marvin