Skip to content

Commit 350d20b

Browse files
committed
LibWeb: Parse HTML fragments directly into DocumentFragment
Refactor the HTML fragment parser to insert the root's children straight into a DocumentFragment instead of a connected temporary document. On top of being a correctness fix, this should also improve performance of fragment parsing. Making this change has other fallout due to the timing of document adoption changing. This results in related other changes needing to be made, including: * The parser having "allow declarative shadow roots" as a parser flag so that it does not read the flag from the wrong document. This prevents some regressions which would happen otherwise, but also aligns our behaviour in general where in some cases the wrong setting was previously being chosen. * Custom element registry is now taken from the *target* node instead of the *context* node. This also prevents regressions as part of the fallout of these changes, but also fixes the selection of the custom element registry for ShadowRoots (which was previously being taken from the root document). * Use a null custom element registry for template.innerHTML. Similar to both cases above, this prevents regressions from the above changes due to the change in timing of document adoption, but fixing this also aligns behaviour closer what other engines implement. Ref: whatwg/html#12624
1 parent 45f7238 commit 350d20b

26 files changed

Lines changed: 503 additions & 420 deletions

Libraries/LibWeb/DOM/Document.cpp

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ WebIDL::ExceptionOr<GC::Ref<Document>> Document::create_and_initialize(Type type
455455
document->m_about_base_url = navigation_params.about_base_url;
456456
document->set_url(*creation_url);
457457
document->m_readiness = HTML::DocumentReadyState::Loading;
458-
document->set_allow_declarative_shadow_roots(true);
458+
document->set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes);
459459
document->set_custom_element_registry(realm.create<HTML::CustomElementRegistry>(realm));
460460

461461
document->m_window = window;
@@ -595,12 +595,7 @@ Document::~Document() = default;
595595
void Document::set_temporary_document_for_fragment_parsing(Badge<HTML::HTMLParser>)
596596
{
597597
// https://html.spec.whatwg.org/multipage/parsing.html#html-fragment-parsing-algorithm
598-
// AD-HOC: The HTML fragment parsing algorithm stages nodes in a temporary document before returning them.
599-
// Treat that document as disconnected so post-connection steps happen only after the fragment is inserted
600-
// into the context document.
601-
// Spec issue: https://github.com/whatwg/html/issues/11023
602598
m_temporary_document_for_fragment_parsing = true;
603-
set_is_connected(false);
604599
}
605600

606601
void Document::set_style_invalidation_counter_dump_interval(Optional<u64> interval)
@@ -1055,16 +1050,23 @@ WebIDL::ExceptionOr<Document*> Document::open(Optional<String> const&, Optional<
10551050
// 15. Set document to no-quirks mode.
10561051
set_quirks_mode(QuirksMode::No);
10571052

1058-
// 16. Create a new HTML parser and associate it with document. This is a script-created parser (meaning that it can be closed by the document.open() and document.close() methods, and that the tokenizer will wait for an explicit call to document.close() before emitting an end-of-file token). The encoding confidence is irrelevant.
1053+
// 16. Create an HTML parser whose allow declarative shadow roots is document's allow declarative shadow roots, and
1054+
// associate it with document. This is a script-created parser (meaning that it can be closed by the document.open()
1055+
// and document.close() methods, and that the tokenizer will wait for an explicit call to document.close() before
1056+
// emitting an end-of-file token). The encoding confidence is irrelevant.
10591057
m_parser = HTML::HTMLParser::create_for_scripting(*this);
1058+
m_parser->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
10601059

10611060
// 17. Set the insertion point to point at just before the end of the input stream (which at this point will be empty).
10621061
m_parser->tokenizer().update_insertion_point();
10631062

1064-
// 18. Update the current document readiness of document to "loading".
1063+
// 18. Set the parser's allow declarative shadow roots to true.
1064+
m_parser->set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes);
1065+
1066+
// 19. Update the current document readiness of document to "loading".
10651067
update_readiness(HTML::DocumentReadyState::Loading);
10661068

1067-
// 19. Return document.
1069+
// 20. Return document.
10681070
return this;
10691071
}
10701072

@@ -5460,11 +5462,6 @@ GC::Ref<DOM::Document> Document::appropriate_template_contents_owner_document()
54605462
if (document_type() == Type::HTML)
54615463
new_document->set_document_type(Type::HTML);
54625464

5463-
// AD-HOC: Copy over the "allow declarative shadow roots" flag, otherwise no elements inside templates will
5464-
// be able to have declarative shadow roots.
5465-
// Spec issue: https://github.com/whatwg/html/issues/11955
5466-
new_document->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
5467-
54685465
// 3. Set document's associated inert template document to newDocument.
54695466
m_associated_inert_template_document = new_document;
54705467
}
@@ -7603,7 +7600,7 @@ Vector<GC::Root<Range>> Document::find_matching_text(String const& query, CaseSe
76037600
}
76047601

76057602
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
7606-
bool Document::allow_declarative_shadow_roots() const
7603+
HTML::HTMLParser::AllowDeclarativeShadowRoots Document::allow_declarative_shadow_roots() const
76077604
{
76087605
return m_allow_declarative_shadow_roots;
76097606
}
@@ -7934,7 +7931,7 @@ void Document::unfullscreen_element(GC::Ref<Element> element)
79347931
}
79357932

79367933
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
7937-
void Document::set_allow_declarative_shadow_roots(bool allow)
7934+
void Document::set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots allow)
79387935
{
79397936
m_allow_declarative_shadow_roots = allow;
79407937
}
@@ -7945,11 +7942,13 @@ void Document::parse_html_from_a_string(Utf16View html)
79457942
// 1. Set document's type to "html".
79467943
set_document_type(DOM::Document::Type::HTML);
79477944

7948-
// 2. Create an HTML parser parser, associated with document.
7945+
// 2. Let parser be a new HTML parser whose allow declarative shadow roots is document's allow declarative shadow roots,
7946+
// associated with document.
79497947
// 3. Place html into the input stream for parser. The encoding confidence is irrelevant.
79507948
// FIXME: We don't have the concept of encoding confidence yet.
79517949
auto scripting_mode = is_scripting_enabled() ? HTML::ParserScriptingMode::Normal : HTML::ParserScriptingMode::Disabled;
79527950
auto parser = HTML::HTMLParser::create_for_decoded_string(*this, html, scripting_mode, "UTF-8"sv);
7951+
parser->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
79537952

79547953
// 4. Start parser and let it run until it has consumed all the characters just inserted into the input stream.
79557954
parser->run(as<HTML::Window>(HTML::relevant_global_object(*this)).associated_document().url());
@@ -7975,7 +7974,7 @@ WebIDL::ExceptionOr<GC::Root<DOM::Document>> Document::parse_html_unsafe(JS::VM&
79757974
document->set_content_type("text/html"_string);
79767975

79777976
// 3. Set document's allow declarative shadow roots to true.
7978-
document->set_allow_declarative_shadow_roots(true);
7977+
document->set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes);
79797978

79807979
// 4. Parse HTML from a string given document and compliantHTML.
79817980
document->parse_html_from_a_string(compliant_html.utf16_view());
@@ -7984,7 +7983,7 @@ WebIDL::ExceptionOr<GC::Root<DOM::Document>> Document::parse_html_unsafe(JS::VM&
79847983
auto& associated_document = as<HTML::Window>(realm.global_object()).associated_document();
79857984
document->set_origin(associated_document.origin());
79867985

7987-
// 5. Return document.
7986+
// 4. Return document.
79887987
return document;
79897988
}
79907989

Libraries/LibWeb/DOM/Document.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include <LibWeb/HTML/DocumentReadyState.h>
4646
#include <LibWeb/HTML/Focus.h>
4747
#include <LibWeb/HTML/PaintConfig.h>
48+
#include <LibWeb/HTML/Parser/HTMLParser.h>
4849
#include <LibWeb/HTML/PreloadEntry.h>
4950
#include <LibWeb/HTML/SandboxingFlagSet.h>
5051
#include <LibWeb/HTML/Scripting/ScriptRegistry.h>
@@ -612,8 +613,8 @@ class WEB_API Document
612613
bool is_fully_active() const;
613614
bool is_active() const;
614615

615-
[[nodiscard]] bool allow_declarative_shadow_roots() const;
616-
void set_allow_declarative_shadow_roots(bool);
616+
[[nodiscard]] HTML::HTMLParser::AllowDeclarativeShadowRoots allow_declarative_shadow_roots() const;
617+
void set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots allow_declarative_shadow_roots);
617618

618619
GC::Ref<HTML::History> history();
619620
GC::Ref<HTML::History> history() const;
@@ -1605,7 +1606,7 @@ class WEB_API Document
16051606
GC::Ptr<HTML::HTMLDialogElement> m_dialog_pointerdown_target;
16061607

16071608
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
1608-
bool m_allow_declarative_shadow_roots { false };
1609+
HTML::HTMLParser::AllowDeclarativeShadowRoots m_allow_declarative_shadow_roots { HTML::HTMLParser::AllowDeclarativeShadowRoots::No };
16091610

16101611
// https://w3c.github.io/selection-api/#dfn-has-scheduled-selectionchange-event
16111612
bool m_has_scheduled_selectionchange_event { false };

Libraries/LibWeb/DOM/DocumentLoading.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ static WebIDL::ExceptionOr<GC::Ref<DOM::Document>> load_html_document(HTML::Navi
126126
}));
127127
}
128128

129-
// 3. Otherwise, create an HTML parser and associate it with the document.
129+
// 3. Otherwise, create an HTML parser whose allow declarative shadow roots is true and associate it with document.
130+
//
130131
// Each task that the networking task source places on the task queue while fetching runs must then fill the
131132
// parser's input byte stream with the fetched bytes and cause the HTML parser to perform the appropriate
132133
// processing of the input stream.
@@ -141,6 +142,7 @@ static WebIDL::ExceptionOr<GC::Ref<DOM::Document>> load_html_document(HTML::Navi
141142
else {
142143
auto body = GC::Ref { *navigation_params.response->body() };
143144
auto parser = HTML::IncrementalDocumentParser::create(document, body, navigation_params.response->url().value(), Fetch::Infrastructure::extract_mime_type(navigation_params.response->header_list()));
145+
parser->set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes);
144146
parser->start();
145147
}
146148

Libraries/LibWeb/DOM/Element.cpp

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
#include <LibWeb/DOM/Attr.h>
5757
#include <LibWeb/DOM/DOMTokenList.h>
5858
#include <LibWeb/DOM/Document.h>
59+
#include <LibWeb/DOM/DocumentFragment.h>
5960
#include <LibWeb/DOM/Element.h>
6061
#include <LibWeb/DOM/ElementFactory.h>
6162
#include <LibWeb/DOM/HTMLCollection.h>
@@ -1539,27 +1540,29 @@ WebIDL::ExceptionOr<void> Element::set_inner_html(TrustedTypes::TrustedHTMLOrStr
15391540
TrustedTypes::InjectionSink::Element_innerHTML,
15401541
TrustedTypes::Script.to_string()));
15411542

1542-
// 2. Let context be this.
1543-
DOM::Node* context = this;
1543+
// 2. Let target be this.
1544+
Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target = GC::Ref { *this };
15441545

1545-
// 3. Let fragment be the result of invoking the fragment parsing algorithm steps with context and compliantString.
1546-
auto fragment = TRY(as<Element>(*context).parse_fragment(compliant_string.utf16_view()));
1547-
1548-
// 4. If context is a template element, then set context to the template element's template contents (a DocumentFragment).
1549-
auto* template_element = as_if<HTML::HTMLTemplateElement>(*context);
1546+
// 3. If target is a template element, then set target to the template element's template contents (a DocumentFragment).
1547+
auto* template_element = as_if<HTML::HTMLTemplateElement>(*this);
15501548
if (template_element)
1551-
context = template_element->content();
1549+
target = template_element->content();
1550+
1551+
// 4. Let fragment be the result of invoking the fragment parsing algorithm steps with target and compliantString.
1552+
auto fragment = TRY(parse_fragment(target, compliant_string.utf16_view()));
15521553

1553-
// 5. Replace all with fragment within context.
1554-
context->replace_all(fragment);
1554+
// 5. Replace all with fragment within target.
1555+
target.visit([&](auto node) {
1556+
node->replace_all(fragment);
1557+
});
15551558

15561559
// NOTE: We don't invalidate style & layout for <template> elements since they don't affect rendering.
15571560
if (!template_element) {
1558-
context->set_needs_style_update(true);
1561+
set_needs_style_update(true);
15591562

1560-
if (context->is_connected()) {
1563+
if (is_connected()) {
15611564
// NOTE: Since the DOM has changed, we have to rebuild the layout tree.
1562-
context->set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
1565+
set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
15631566
}
15641567
}
15651568

@@ -2666,32 +2669,18 @@ bool Element::is_actually_disabled() const
26662669
}
26672670

26682671
// https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#fragment-parsing-algorithm-steps
2669-
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(Utf16View markup, HTML::ParserScriptingMode scripting_mode)
2672+
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target, Utf16View markup, HTML::ParserScriptingMode scripting_mode)
26702673
{
26712674
// 1. Assert: scriptingMode is either Inert or Fragment.
26722675
VERIFY(scripting_mode == HTML::ParserScriptingMode::Inert || scripting_mode == HTML::ParserScriptingMode::Fragment);
26732676

2674-
// 2. Let newChildren be null.
2675-
Vector<GC::Root<Node>> new_children;
2676-
2677-
// 3. If context's node document is an XML document, then set newChildren to the result of invoking the XML fragment parsing algorithm given context and markup.
2678-
if (document().is_xml_document()) {
2679-
new_children = TRY(XMLFragmentParser::parse_xml_fragment(*this, markup));
2680-
}
2681-
// 4. Otherwise, set newChildren to the result of invoking the HTML fragment parsing algorithm given context, markup, false, and scriptingMode.
2682-
else {
2683-
new_children = TRY(HTML::HTMLParser::parse_html_fragment(*this, markup, HTML::HTMLParser::AllowDeclarativeShadowRoots::No, scripting_mode));
2684-
}
2685-
2686-
// 5. Let fragment be a new DocumentFragment whose node document is context's node document.
2687-
auto fragment = realm().create<DOM::DocumentFragment>(document());
2688-
2689-
// 6. For each node of newChildren, in tree order: append node to fragment.
2690-
for (auto& child : new_children)
2691-
TRY(fragment->append_child(*child));
2677+
// 2. If target's node document is an XML document, then return the result of invoking the XML fragment parsing
2678+
// algorithm given target and markup.
2679+
if (target.visit([](auto node) { return node->document().is_xml_document(); }))
2680+
return XMLFragmentParser::parse_xml_fragment(target, markup);
26922681

2693-
// 7. Return fragment.
2694-
return fragment;
2682+
// 3. Return the result of invoking the HTML fragment parsing algorithm given context, markup, false, and scriptingMode.
2683+
return HTML::HTMLParser::parse_html_fragment(target, markup, HTML::HTMLParser::AllowDeclarativeShadowRoots::No, scripting_mode);
26952684
}
26962685

26972686
// https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dom-element-outerhtml
@@ -2728,7 +2717,7 @@ WebIDL::ExceptionOr<void> Element::set_outer_html(TrustedTypes::TrustedHTMLOrStr
27282717
parent = TRY(create_element(document(), HTML::TagNames::body, Namespace::HTML));
27292718

27302719
// 6. Let fragment be the result of invoking the fragment parsing algorithm steps given parent and compliantString.
2731-
auto fragment = TRY(as<Element>(*parent).parse_fragment(compliant_string.utf16_view()));
2720+
auto fragment = TRY(Element::parse_fragment(GC::Ref { as<Element>(*parent) }, compliant_string.utf16_view()));
27322721

27332722
// 6. Replace this with fragment within this's parent.
27342723
TRY(this->parent()->replace_child(fragment, *this));
@@ -2789,7 +2778,7 @@ WebIDL::ExceptionOr<void> Element::insert_adjacent_html(String const& position,
27892778
}
27902779

27912780
// 5. Let fragment be the result of invoking the fragment parsing algorithm steps with context and compliantString.
2792-
auto fragment = TRY(as<Element>(*context).parse_fragment(compliant_string.utf16_view()));
2781+
auto fragment = TRY(Element::parse_fragment(GC::Ref { as<Element>(*context) }, compliant_string.utf16_view()));
27932782

27942783
// 6. Use the first matching item from this list:
27952784

@@ -4893,12 +4882,12 @@ WebIDL::ExceptionOr<void> Element::set_html_unsafe(TrustedTypes::TrustedHTMLOrSt
48934882
TrustedTypes::Script.to_string()));
48944883

48954884
// 2. Let target be this's template contents if this is a template element; otherwise this.
4896-
DOM::Node* target = this;
4885+
Variant<GC::Ref<DOM::Element>, GC::Ref<DOM::DocumentFragment>> target = GC::Ref { *this };
48974886
if (is<HTML::HTMLTemplateElement>(*this))
4898-
target = as<HTML::HTMLTemplateElement>(*this).content().ptr();
4887+
target = as<HTML::HTMLTemplateElement>(*this).content();
48994888

49004889
// 3. Unsafe set HTML given target, this, and compliantHTML.
4901-
TRY(target->unsafely_set_html(*this, compliant_html.utf16_view()));
4890+
TRY(unsafely_set_html(target, compliant_html.utf16_view()));
49024891

49034892
return {};
49044893
}

Libraries/LibWeb/DOM/Element.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ class WEB_API Element
284284
CSS::StyleSheetList& document_or_shadow_root_style_sheets();
285285
ElementByIdMap& document_or_shadow_root_element_by_id_map();
286286

287-
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> parse_fragment(Utf16View markup, HTML::ParserScriptingMode = HTML::ParserScriptingMode::Inert);
287+
static WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> parse_fragment(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target, Utf16View markup, HTML::ParserScriptingMode = HTML::ParserScriptingMode::Inert);
288288

289289
[[nodiscard]] GC::Ptr<Element const> element_to_inherit_style_from(Optional<CSS::PseudoElement>) const;
290290

Libraries/LibWeb/DOM/Node.cpp

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2309,21 +2309,16 @@ WebIDL::ExceptionOr<Utf16String> Node::serialize_fragment(HTML::RequireWellForme
23092309
}
23102310

23112311
// https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#unsafely-set-html
2312-
WebIDL::ExceptionOr<void> Node::unsafely_set_html(Element& context_element, Utf16View html)
2312+
WebIDL::ExceptionOr<void> Node::unsafely_set_html(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target, Utf16View html)
23132313
{
2314-
// 1. Let newChildren be the result of the HTML fragment parsing algorithm given contextElement, html, and true.
2315-
auto new_children = TRY(HTML::HTMLParser::parse_html_fragment(context_element, html, HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes));
2314+
// FIXME: Update for sanitizer API.
2315+
// 7. Let fragment be the result of invoking the HTML fragment parsing algorithm given target, html, true, and scriptingMode.
2316+
auto fragment = TRY(HTML::HTMLParser::parse_html_fragment(target, html, HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes));
23162317

2317-
// 2. Let fragment be a new DocumentFragment whose node document is contextElement’s node document.
2318-
auto fragment = realm().create<DocumentFragment>(context_element.document());
2319-
2320-
// 3. For each node in newChildren, append node to fragment.
2321-
for (auto& child : new_children)
2322-
// I don't know if this can throw here, but let's be safe.
2323-
(void)TRY(fragment->append_child(*child));
2324-
2325-
// 4. Replace all with fragment within contextElement.
2326-
replace_all(fragment);
2318+
// 9. Replace all with fragment within target.
2319+
target.visit([&](auto node) {
2320+
node->replace_all(fragment);
2321+
});
23272322

23282323
return {};
23292324
}

Libraries/LibWeb/DOM/Node.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ class WEB_API Node : public EventTarget
426426

427427
WebIDL::ExceptionOr<Utf16String> serialize_fragment(HTML::RequireWellFormed, FragmentSerializationMode = FragmentSerializationMode::Inner) const;
428428

429-
WebIDL::ExceptionOr<void> unsafely_set_html(Element&, Utf16View);
429+
WebIDL::ExceptionOr<void> unsafely_set_html(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>>, Utf16View);
430430

431431
void replace_all(GC::Ptr<Node>);
432432
void string_replace_all(Utf16String);

Libraries/LibWeb/DOM/Range.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1345,7 +1345,7 @@ WebIDL::ExceptionOr<GC::Ref<DocumentFragment>> Range::create_contextual_fragment
13451345
}
13461346

13471347
// 7. Return the result of invoking the fragment parsing algorithm steps with element, compliantString, and Fragment.
1348-
return element->parse_fragment(compliant_string.utf16_view(), HTML::ParserScriptingMode::Fragment);
1348+
return Element::parse_fragment(GC::Ref { *element }, compliant_string.utf16_view(), HTML::ParserScriptingMode::Fragment);
13491349
}
13501350

13511351
}

0 commit comments

Comments
 (0)