Skip to content

Commit 81ccdc0

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 d0bdb31 commit 81ccdc0

26 files changed

Lines changed: 506 additions & 401 deletions

Libraries/LibWeb/DOM/Document.cpp

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

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

605600
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

@@ -5480,11 +5482,6 @@ GC::Ref<DOM::Document> Document::appropriate_template_contents_owner_document()
54805482
if (document_type() == Type::HTML)
54815483
new_document->set_document_type(Type::HTML);
54825484

5483-
// AD-HOC: Copy over the "allow declarative shadow roots" flag, otherwise no elements inside templates will
5484-
// be able to have declarative shadow roots.
5485-
// Spec issue: https://github.com/whatwg/html/issues/11955
5486-
new_document->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
5487-
54885485
// 3. Set document's associated inert template document to newDocument.
54895486
m_associated_inert_template_document = new_document;
54905487
}
@@ -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(StringView 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());
@@ -7974,17 +7973,14 @@ WebIDL::ExceptionOr<GC::Root<DOM::Document>> Document::parse_html_unsafe(JS::VM&
79747973
auto document = Document::create(realm);
79757974
document->set_content_type("text/html"_string);
79767975

7977-
// 3. Set document's allow declarative shadow roots to true.
7978-
document->set_allow_declarative_shadow_roots(true);
7979-
7980-
// 4. Parse HTML from a string given document and compliantHTML.
7976+
// 3. Parse HTML from a string given document and compliantHTML.
79817977
document->parse_html_from_a_string(compliant_html.to_utf8_but_should_be_ported_to_utf16());
79827978

79837979
// AD-HOC: Setting the origin to match that of the associated document matches the behavior of existing browsers.
79847980
auto& associated_document = as<HTML::Window>(realm.global_object()).associated_document();
79857981
document->set_origin(associated_document.origin());
79867982

7987-
// 5. Return document.
7983+
// 4. Return document.
79887984
return document;
79897985
}
79907986

Libraries/LibWeb/DOM/Document.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include <LibWeb/HTML/DocumentReadyState.h>
4545
#include <LibWeb/HTML/Focus.h>
4646
#include <LibWeb/HTML/PaintConfig.h>
47+
#include <LibWeb/HTML/Parser/HTMLParser.h>
4748
#include <LibWeb/HTML/PreloadEntry.h>
4849
#include <LibWeb/HTML/SandboxingFlagSet.h>
4950
#include <LibWeb/HTML/Scripting/ScriptRegistry.h>
@@ -609,8 +610,8 @@ class WEB_API Document
609610
bool is_fully_active() const;
610611
bool is_active() const;
611612

612-
[[nodiscard]] bool allow_declarative_shadow_roots() const;
613-
void set_allow_declarative_shadow_roots(bool);
613+
[[nodiscard]] HTML::HTMLParser::AllowDeclarativeShadowRoots allow_declarative_shadow_roots() const;
614+
void set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots allow_declarative_shadow_roots);
614615

615616
GC::Ref<HTML::History> history();
616617
GC::Ref<HTML::History> history() const;
@@ -1599,7 +1600,7 @@ class WEB_API Document
15991600
GC::Ptr<HTML::HTMLDialogElement> m_dialog_pointerdown_target;
16001601

16011602
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
1602-
bool m_allow_declarative_shadow_roots { false };
1603+
HTML::HTMLParser::AllowDeclarativeShadowRoots m_allow_declarative_shadow_roots { HTML::HTMLParser::AllowDeclarativeShadowRoots::No };
16031604

16041605
// https://w3c.github.io/selection-api/#dfn-has-scheduled-selectionchange-event
16051606
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>
@@ -1534,27 +1535,29 @@ WebIDL::ExceptionOr<void> Element::set_inner_html(TrustedTypes::TrustedHTMLOrStr
15341535
TrustedTypes::InjectionSink::Element_innerHTML,
15351536
TrustedTypes::Script.to_string()));
15361537

1537-
// 2. Let context be this.
1538-
DOM::Node* context = this;
1538+
// 2. Let target be this.
1539+
Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target = GC::Ref { *this };
15391540

1540-
// 3. Let fragment be the result of invoking the fragment parsing algorithm steps with context and compliantString.
1541-
auto fragment = TRY(as<Element>(*context).parse_fragment(compliant_string.to_utf8_but_should_be_ported_to_utf16()));
1542-
1543-
// 4. If context is a template element, then set context to the template element's template contents (a DocumentFragment).
1544-
auto* template_element = as_if<HTML::HTMLTemplateElement>(*context);
1541+
// 3. If target is a template element, then set target to the template element's template contents (a DocumentFragment).
1542+
auto* template_element = as_if<HTML::HTMLTemplateElement>(*this);
15451543
if (template_element)
1546-
context = template_element->content();
1544+
target = template_element->content();
1545+
1546+
// 4. Let fragment be the result of invoking the fragment parsing algorithm steps with target and compliantString.
1547+
auto fragment = TRY(parse_fragment(target, compliant_string.to_utf8_but_should_be_ported_to_utf16()));
15471548

1548-
// 5. Replace all with fragment within context.
1549-
context->replace_all(fragment);
1549+
// 5. Replace all with fragment within target.
1550+
target.visit([&](auto node) {
1551+
node->replace_all(fragment);
1552+
});
15501553

15511554
// NOTE: We don't invalidate style & layout for <template> elements since they don't affect rendering.
15521555
if (!template_element) {
1553-
context->set_needs_style_update(true);
1556+
set_needs_style_update(true);
15541557

1555-
if (context->is_connected()) {
1558+
if (is_connected()) {
15561559
// NOTE: Since the DOM has changed, we have to rebuild the layout tree.
1557-
context->set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
1560+
set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
15581561
}
15591562
}
15601563

@@ -2661,32 +2664,18 @@ bool Element::is_actually_disabled() const
26612664
}
26622665

26632666
// https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#fragment-parsing-algorithm-steps
2664-
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(StringView markup, HTML::ParserScriptingMode scripting_mode)
2667+
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target, StringView markup, HTML::ParserScriptingMode scripting_mode)
26652668
{
26662669
// 1. Assert: scriptingMode is either Inert or Fragment.
26672670
VERIFY(scripting_mode == HTML::ParserScriptingMode::Inert || scripting_mode == HTML::ParserScriptingMode::Fragment);
26682671

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

2688-
// 7. Return fragment.
2689-
return fragment;
2677+
// 3. Return the result of invoking the HTML fragment parsing algorithm given context, markup, false, and scriptingMode.
2678+
return HTML::HTMLParser::parse_html_fragment(target, markup, HTML::HTMLParser::AllowDeclarativeShadowRoots::No, scripting_mode);
26902679
}
26912680

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

27252714
// 6. Let fragment be the result of invoking the fragment parsing algorithm steps given parent and compliantString.
2726-
auto fragment = TRY(as<Element>(*parent).parse_fragment(compliant_string.to_utf8_but_should_be_ported_to_utf16()));
2715+
auto fragment = TRY(Element::parse_fragment(GC::Ref { as<Element>(*parent) }, compliant_string.to_utf8_but_should_be_ported_to_utf16()));
27272716

27282717
// 6. Replace this with fragment within this's parent.
27292718
TRY(this->parent()->replace_child(fragment, *this));
@@ -2784,7 +2773,7 @@ WebIDL::ExceptionOr<void> Element::insert_adjacent_html(String const& position,
27842773
}
27852774

27862775
// 5. Let fragment be the result of invoking the fragment parsing algorithm steps with context and compliantString.
2787-
auto fragment = TRY(as<Element>(*context).parse_fragment(compliant_string.to_utf8_but_should_be_ported_to_utf16()));
2776+
auto fragment = TRY(Element::parse_fragment(GC::Ref { as<Element>(*context) }, compliant_string.to_utf8_but_should_be_ported_to_utf16()));
27882777

27892778
// 6. Use the first matching item from this list:
27902779

@@ -4888,12 +4877,12 @@ WebIDL::ExceptionOr<void> Element::set_html_unsafe(TrustedTypes::TrustedHTMLOrSt
48884877
TrustedTypes::Script.to_string()));
48894878

48904879
// 2. Let target be this's template contents if this is a template element; otherwise this.
4891-
DOM::Node* target = this;
4880+
Variant<GC::Ref<DOM::Element>, GC::Ref<DOM::DocumentFragment>> target = GC::Ref { *this };
48924881
if (is<HTML::HTMLTemplateElement>(*this))
4893-
target = as<HTML::HTMLTemplateElement>(*this).content().ptr();
4882+
target = as<HTML::HTMLTemplateElement>(*this).content();
48944883

48954884
// 3. Unsafe set HTML given target, this, and compliantHTML.
4896-
TRY(target->unsafely_set_html(*this, compliant_html.to_utf8_but_should_be_ported_to_utf16()));
4885+
TRY(unsafely_set_html(target, compliant_html.to_utf8_but_should_be_ported_to_utf16()));
48974886

48984887
return {};
48994888
}

Libraries/LibWeb/DOM/Element.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ class WEB_API Element
293293
CSS::StyleSheetList& document_or_shadow_root_style_sheets();
294294
ElementByIdMap& document_or_shadow_root_element_by_id_map();
295295

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

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

Libraries/LibWeb/DOM/Node.cpp

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2279,22 +2279,17 @@ WebIDL::ExceptionOr<Utf16String> Node::serialize_fragment(HTML::RequireWellForme
22792279
return Utf16String::from_utf8(TRY(HTML::serialize_node_to_xml_string(*this, require_well_formed)));
22802280
}
22812281

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

2288-
// 2. Let fragment be a new DocumentFragment whose node document is contextElement’s node document.
2289-
auto fragment = realm().create<DocumentFragment>(context_element.document());
2290-
2291-
// 3. For each node in newChildren, append node to fragment.
2292-
for (auto& child : new_children)
2293-
// I don't know if this can throw here, but let's be safe.
2294-
(void)TRY(fragment->append_child(*child));
2295-
2296-
// 4. Replace all with fragment within contextElement.
2297-
replace_all(fragment);
2289+
// 9. Replace all with fragment within target.
2290+
target.visit([&](auto node) {
2291+
node->replace_all(fragment);
2292+
});
22982293

22992294
return {};
23002295
}

Libraries/LibWeb/DOM/Node.h

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

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

423-
WebIDL::ExceptionOr<void> unsafely_set_html(Element&, StringView);
423+
WebIDL::ExceptionOr<void> unsafely_set_html(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>>, StringView);
424424

425425
void replace_all(GC::Ptr<Node>);
426426
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.to_utf8_but_should_be_ported_to_utf16(), HTML::ParserScriptingMode::Fragment);
1348+
return Element::parse_fragment(GC::Ref { *element }, compliant_string.to_utf8_but_should_be_ported_to_utf16(), HTML::ParserScriptingMode::Fragment);
13491349
}
13501350

13511351
}

0 commit comments

Comments
 (0)