[PATCH v2 0/3] MR1918: Implement HTMLDocument_get_body and HTMLDocument_get_all for document fragments
This functions should also work with document fragments, in wine they only return NULL.
-- v2: mshtml: Implement HTMLDocument_get_body for document fragments.
From: Santino Mazza smazza@codeweavers.com
--- dlls/mshtml/tests/dom.c | 48 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index 6a960bfa21f..91bc73fd2d0 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -10984,7 +10984,7 @@ static IHTMLDocument2 *create_docfrag(IHTMLDocument2 *doc) static void test_docfrag(IHTMLDocument2 *doc) { IHTMLDocument2 *frag, *owner_doc, *doc_node; - IHTMLElement *div, *body, *br; + IHTMLElement *div, *body, *br,*html; IHTMLElementCollection *col; IHTMLLocation *location; HRESULT hres; @@ -10998,6 +10998,13 @@ static void test_docfrag(IHTMLDocument2 *doc) ET_BR };
+ static const elem_type_t frag_types[] = { + ET_HTML, + ET_DIV, + ET_DIV, + ET_BODY + }; + frag = create_docfrag(doc);
test_disp((IUnknown*)frag, &DIID_DispHTMLDocument, &CLSID_HTMLDocument, L"[object]"); @@ -11030,6 +11037,45 @@ static void test_docfrag(IHTMLDocument2 *doc) test_elem_collection((IUnknown*)col, all_types, ARRAY_SIZE(all_types)); IHTMLElementCollection_Release(col);
+ html = test_create_elem(doc, L"HTML"); + test_elem_source_index(html, -1); + test_node_append_child((IUnknown*)frag, (IUnknown*)html); + test_elem_source_index(html, 0); + + div = test_create_elem(doc, L"DIV"); + test_elem_source_index(div, -1); + test_node_append_child((IUnknown*)html, (IUnknown*)div); + test_elem_source_index(div, 1); + IHTMLElement_Release(div); + + div = test_create_elem(doc, L"DIV"); + test_elem_source_index(div, -1); + test_node_append_child((IUnknown*)html, (IUnknown*)div); + test_elem_source_index(div, 2); + + body = test_create_elem(doc, L"BODY"); + test_elem_source_index(body, -1); + test_node_append_child((IUnknown*)div, (IUnknown*)body); + test_elem_source_index(body, 3); + IHTMLElement_Release(body); + IHTMLElement_Release(div); + IHTMLElement_Release(html); + + hres = IHTMLDocument2_get_body(frag, &body); + ok(hres == S_OK, "get_body failed: %08lx\n", hres); + todo_wine ok(body != NULL, "body == NULL\n"); + if (body) + IHTMLElement_Release(body); + + col = NULL; + hres = IHTMLDocument2_get_all(frag, &col); + todo_wine ok(hres == S_OK, "get_all failed: %08lx\n", hres); + todo_wine ok(col != NULL, "got null elements collection\n"); + if (col) { + test_elem_collection((IUnknown *) col, frag_types, ARRAY_SIZE(frag_types)); + IHTMLElementCollection_Release(col); + } + div = test_create_elem(frag, L"div"); owner_doc = get_owner_doc((IUnknown*)div); doc_node = get_doc_node(doc);
From: Santino Mazza smazza@codeweavers.com
--- dlls/mshtml/htmldoc.c | 4 ++-- dlls/mshtml/tests/dom.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 90c4706abee..ac471e9e1a8 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -495,8 +495,8 @@ static HRESULT WINAPI HTMLDocument_get_all(IHTMLDocument2 *iface, IHTMLElementCo TRACE("(%p)->(%p)\n", This, p);
if(!This->dom_document) { - WARN("NULL dom_document\n"); - return E_UNEXPECTED; + *p = create_all_collection(&This->node, FALSE); + return S_OK; }
nsres = nsIDOMDocument_GetDocumentElement(This->dom_document, &nselem); diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index 91bc73fd2d0..a726801a43e 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -11069,8 +11069,8 @@ static void test_docfrag(IHTMLDocument2 *doc)
col = NULL; hres = IHTMLDocument2_get_all(frag, &col); - todo_wine ok(hres == S_OK, "get_all failed: %08lx\n", hres); - todo_wine ok(col != NULL, "got null elements collection\n"); + ok(hres == S_OK, "get_all failed: %08lx\n", hres); + ok(col != NULL, "got null elements collection\n"); if (col) { test_elem_collection((IUnknown *) col, frag_types, ARRAY_SIZE(frag_types)); IHTMLElementCollection_Release(col);
From: Santino Mazza smazza@codeweavers.com
--- dlls/mshtml/htmldoc.c | 8 ++++++++ dlls/mshtml/tests/dom.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index ac471e9e1a8..fda47690ddb 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -538,6 +538,14 @@ static HRESULT WINAPI HTMLDocument_get_body(IHTMLDocument2 *iface, IHTMLElement return E_UNEXPECTED; } } + else if(This->node.nsnode) { + nsAString nsnode_name; + + nsAString_Init(&nsnode_name, L"BODY"); + nsIDOMDocumentFragment_QuerySelector((nsIDOMDocumentFragment*)This->node.nsnode, + &nsnode_name, (nsIDOMElement**)&nsbody); + nsAString_Finish(&nsnode_name); + }
if(!nsbody) { *p = NULL; diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index a726801a43e..eea3dbe7126 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -11063,7 +11063,7 @@ static void test_docfrag(IHTMLDocument2 *doc)
hres = IHTMLDocument2_get_body(frag, &body); ok(hres == S_OK, "get_body failed: %08lx\n", hres); - todo_wine ok(body != NULL, "body == NULL\n"); + ok(body != NULL, "body == NULL\n"); if (body) IHTMLElement_Release(body);
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128195
Your paranoid android.
=== debian11 (32 bit report) ===
gdi32: Unhandled exception: page fault on read access to 0x7e4370d8 in 32-bit code (0xf7ebaf93).
Changed to draft because of code freeze.
On Fri Jan 6 16:00:44 2023 +0000, Santino Mazza wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/1918/diffs?diff_id=26429&start_sha=4d50f390f1af4d328207dfcee97b9a51dda216b3#de52190fefc7d08534f3a8d527401e2e34decdeb_567_523)
Rather than cast, the correct way is to QueryInterface for it. :)
On Fri Jan 6 16:47:36 2023 +0000, Gabriel Ivăncescu wrote:
Rather than cast, the correct way is to QueryInterface for it. :)
Also, use `nsAString_InitDepend` instead of nsAString_Init; it basically makes it depend on the source (reference it), instead of copying it, and well the source is a constant string so it's always available.
And there is no reason to check for nsnode (`This->node.nsnode`), it should never be NULL. It does get NULL when it's unlinked, but unlinked means that nothing else references it, so if it ends up here it's a (refcount) bug somewhere.
On Fri Jan 6 16:52:37 2023 +0000, Gabriel Ivăncescu wrote:
Also, use `nsAString_InitDepend` instead of nsAString_Init; it basically makes it depend on the source (reference it), instead of copying it, and well the source is a constant string so it's always available. And there is no reason to check for nsnode (`This->node.nsnode`), it should never be NULL. It does get NULL when it's unlinked, but unlinked means that nothing else references it, so if it ends up here it's a (refcount) bug somewhere.
Oh, I used casting because in HTMLDocument3_createDocumentFragment it uses casting for passing nsIDOMDocumentFragment to create_document_fragment as nsIDOMNode
-
Gabriel Ivăncescu (＠insn) -
Marvin -
Santino Mazza
-
Santino Mazza (＠tati1454)