Skip to content

Commit fc23bfb

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 7dffe07 commit fc23bfb

26 files changed

Lines changed: 504 additions & 397 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

@@ -5456,11 +5458,6 @@ GC::Ref<DOM::Document> Document::appropriate_template_contents_owner_document()
54565458
if (document_type() == Type::HTML)
54575459
new_document->set_document_type(Type::HTML);
54585460

5459-
// AD-HOC: Copy over the "allow declarative shadow roots" flag, otherwise no elements inside templates will
5460-
// be able to have declarative shadow roots.
5461-
// Spec issue: https://github.com/whatwg/html/issues/11955
5462-
new_document->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
5463-
54645461
// 3. Set document's associated inert template document to newDocument.
54655462
m_associated_inert_template_document = new_document;
54665463
}
@@ -7599,7 +7596,7 @@ Vector<GC::Root<Range>> Document::find_matching_text(String const& query, CaseSe
75997596
}
76007597

76017598
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
7602-
bool Document::allow_declarative_shadow_roots() const
7599+
HTML::HTMLParser::AllowDeclarativeShadowRoots Document::allow_declarative_shadow_roots() const
76037600
{
76047601
return m_allow_declarative_shadow_roots;
76057602
}
@@ -7930,7 +7927,7 @@ void Document::unfullscreen_element(GC::Ref<Element> element)
79307927
}
79317928

79327929
// https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
7933-
void Document::set_allow_declarative_shadow_roots(bool allow)
7930+
void Document::set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots allow)
79347931
{
79357932
m_allow_declarative_shadow_roots = allow;
79367933
}
@@ -7941,11 +7938,13 @@ void Document::parse_html_from_a_string(Utf16View html)
79417938
// 1. Set document's type to "html".
79427939
set_document_type(DOM::Document::Type::HTML);
79437940

7944-
// 2. Create an HTML parser parser, associated with document.
7941+
// 2. Let parser be a new HTML parser whose allow declarative shadow roots is document's allow declarative shadow roots,
7942+
// associated with document.
79457943
// 3. Place html into the input stream for parser. The encoding confidence is irrelevant.
79467944
// FIXME: We don't have the concept of encoding confidence yet.
79477945
auto scripting_mode = is_scripting_enabled() ? HTML::ParserScriptingMode::Normal : HTML::ParserScriptingMode::Disabled;
79487946
auto parser = HTML::HTMLParser::create_for_decoded_string(*this, html, scripting_mode, "UTF-8"sv);
7947+
parser->set_allow_declarative_shadow_roots(allow_declarative_shadow_roots());
79497948

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

79737972
// 3. Set document's allow declarative shadow roots to true.
7974-
document->set_allow_declarative_shadow_roots(true);
7973+
document->set_allow_declarative_shadow_roots(HTML::HTMLParser::AllowDeclarativeShadowRoots::Yes);
79757974

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

7983-
// 5. Return document.
7982+
// 4. Return document.
79847983
return document;
79857984
}
79867985

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>
@@ -1523,27 +1524,29 @@ WebIDL::ExceptionOr<void> Element::set_inner_html(TrustedTypes::TrustedHTMLOrStr
15231524
TrustedTypes::InjectionSink::Element_innerHTML,
15241525
TrustedTypes::Script.to_string()));
15251526

1526-
// 2. Let context be this.
1527-
DOM::Node* context = this;
1527+
// 2. Let target be this.
1528+
Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target = GC::Ref { *this };
15281529

1529-
// 3. Let fragment be the result of invoking the fragment parsing algorithm steps with context and compliantString.
1530-
auto fragment = TRY(as<Element>(*context).parse_fragment(compliant_string.utf16_view()));
1531-
1532-
// 4. If context is a template element, then set context to the template element's template contents (a DocumentFragment).
1533-
auto* template_element = as_if<HTML::HTMLTemplateElement>(*context);
1530+
// 3. If target is a template element, then set target to the template element's template contents (a DocumentFragment).
1531+
auto* template_element = as_if<HTML::HTMLTemplateElement>(*this);
15341532
if (template_element)
1535-
context = template_element->content();
1533+
target = template_element->content();
1534+
1535+
// 4. Let fragment be the result of invoking the fragment parsing algorithm steps with target and compliantString.
1536+
auto fragment = TRY(parse_fragment(target, compliant_string.utf16_view()));
15361537

1537-
// 5. Replace all with fragment within context.
1538-
context->replace_all(fragment);
1538+
// 5. Replace all with fragment within target.
1539+
target.visit([&](auto node) {
1540+
node->replace_all(fragment);
1541+
});
15391542

15401543
// NOTE: We don't invalidate style & layout for <template> elements since they don't affect rendering.
15411544
if (!template_element) {
1542-
context->set_needs_style_update(true);
1545+
set_needs_style_update(true);
15431546

1544-
if (context->is_connected()) {
1547+
if (is_connected()) {
15451548
// NOTE: Since the DOM has changed, we have to rebuild the layout tree.
1546-
context->set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
1549+
set_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::ElementSetInnerHTML);
15471550
}
15481551
}
15491552

@@ -2650,32 +2653,18 @@ bool Element::is_actually_disabled() const
26502653
}
26512654

26522655
// https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#fragment-parsing-algorithm-steps
2653-
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(Utf16View markup, HTML::ParserScriptingMode scripting_mode)
2656+
WebIDL::ExceptionOr<GC::Ref<DOM::DocumentFragment>> Element::parse_fragment(Variant<GC::Ref<Element>, GC::Ref<DocumentFragment>> target, Utf16View markup, HTML::ParserScriptingMode scripting_mode)
26542657
{
26552658
// 1. Assert: scriptingMode is either Inert or Fragment.
26562659
VERIFY(scripting_mode == HTML::ParserScriptingMode::Inert || scripting_mode == HTML::ParserScriptingMode::Fragment);
26572660

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

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

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

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

27172706
// 6. Replace this with fragment within this's parent.
27182707
TRY(this->parent()->replace_child(fragment, *this));
@@ -2773,7 +2762,7 @@ WebIDL::ExceptionOr<void> Element::insert_adjacent_html(String const& position,
27732762
}
27742763

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

27782767
// 6. Use the first matching item from this list:
27792768

@@ -4877,12 +4866,12 @@ WebIDL::ExceptionOr<void> Element::set_html_unsafe(TrustedTypes::TrustedHTMLOrSt
48774866
TrustedTypes::Script.to_string()));
48784867

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

48844873
// 3. Unsafe set HTML given target, this, and compliantHTML.
4885-
TRY(target->unsafely_set_html(*this, compliant_html.utf16_view()));
4874+
TRY(unsafely_set_html(target, compliant_html.utf16_view()));
48864875

48874876
return {};
48884877
}

Libraries/LibWeb/DOM/Element.h

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

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

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

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)