Skip to content

Commit

Permalink
LibWeb: Look at both namespace & tag name in HTML parser stack checks
Browse files Browse the repository at this point in the history
We were neglecting to check the namespace when looking for a specific
type of element on the stack of open elements in many cases.

This caused us to confuse HTML and SVG elements.
  • Loading branch information
awesomekling committed Nov 5, 2024
1 parent 7d7f8f1 commit ceedfb3
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ Rerun

Found 1 tests

1 Fail
1 Pass
Details
Result Test Name MessageFail html5lib_namespace-sensitivity.html de0a2051123e97a540e3aeb58375103bda021122
Result Test Name MessagePass html5lib_namespace-sensitivity.html de0a2051123e97a540e3aeb58375103bda021122
23 changes: 13 additions & 10 deletions Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ JS::NonnullGCPtr<DOM::Element> HTMLParser::create_element_for(HTMLToken const& t
auto& html_element = form_associated_element->form_associated_element_to_html_element();

if (m_form_element.ptr()
&& !m_stack_of_open_elements.contains(HTML::TagNames::template_)
&& !m_stack_of_open_elements.contains_template_element()
&& (!form_associated_element->is_listed() || !html_element.has_attribute(HTML::AttributeNames::form))
&& &intended_parent.root() == &m_form_element->root()) {
form_associated_element->set_form(m_form_element.ptr());
Expand Down Expand Up @@ -1056,7 +1056,7 @@ void HTMLParser::handle_in_head(HTMLToken& token)
// -> An end tag whose tag name is "template"
if (token.is_end_tag() && token.tag_name() == HTML::TagNames::template_) {
// If there is no template element on the stack of open elements, then this is a parse error; ignore the token.
if (!m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
if (!m_stack_of_open_elements.contains_template_element()) {
log_parse_error();
return;
}
Expand Down Expand Up @@ -1768,7 +1768,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)
log_parse_error();

// If there is a template element on the stack of open elements, then ignore the token.
if (m_stack_of_open_elements.contains(HTML::TagNames::template_))
if (m_stack_of_open_elements.contains_template_element())
return;

// Otherwise, for each attribute on the token, check to see if the attribute is already present on the top element of the stack of open elements.
Expand Down Expand Up @@ -1806,7 +1806,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)
// (fragment case or there is a template element on the stack)
if (m_stack_of_open_elements.elements().size() == 1
|| m_stack_of_open_elements.elements().at(1)->local_name() != HTML::TagNames::body
|| m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
|| m_stack_of_open_elements.contains_template_element()) {
return;
}

Expand All @@ -1832,7 +1832,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)
// (fragment case or there is a template element on the stack)
if (m_stack_of_open_elements.elements().size() == 1
|| m_stack_of_open_elements.elements().at(1)->local_name() != HTML::TagNames::body) {
VERIFY(m_parsing_fragment || m_stack_of_open_elements.contains(HTML::TagNames::template_));
VERIFY(m_parsing_fragment || m_stack_of_open_elements.contains_template_element());
return;
}

Expand Down Expand Up @@ -1986,7 +1986,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)
// -> A start tag whose tag name is "form"
if (token.is_start_tag() && token.tag_name() == HTML::TagNames::form) {
// If the form element pointer is not null, and there is no template element on the stack of open elements, then this is a parse error; ignore the token.
if (m_form_element.ptr() && !m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
if (m_form_element.ptr() && !m_stack_of_open_elements.contains_template_element()) {
log_parse_error();
return;
}
Expand All @@ -1998,7 +1998,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)

// Insert an HTML element for the token, and, if there is no template element on the stack of open elements, set the form element pointer to point to the element created.
auto element = insert_html_element(token);
if (!m_stack_of_open_elements.contains(HTML::TagNames::template_))
if (!m_stack_of_open_elements.contains_template_element())
m_form_element = verify_cast<HTMLFormElement>(*element);
return;
}
Expand Down Expand Up @@ -2163,7 +2163,7 @@ void HTMLParser::handle_in_body(HTMLToken& token)
// -> An end tag whose tag name is "form"
if (token.is_end_tag() && token.tag_name() == HTML::TagNames::form) {
// If there is no template element on the stack of open elements, then run these substeps:
if (!m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
if (!m_stack_of_open_elements.contains_template_element()) {
// 1. Let node be the element that the form element pointer is set to, or null if it is not set to an element.
auto node = m_form_element;

Expand Down Expand Up @@ -3521,7 +3521,7 @@ void HTMLParser::handle_in_table(HTMLToken& token)

// If there is a template element on the stack of open elements,
// or if the form element pointer is not null, ignore the token.
if (m_form_element.ptr() || m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
if (m_form_element.ptr() || m_stack_of_open_elements.contains_template_element()) {
return;
}

Expand Down Expand Up @@ -3939,7 +3939,7 @@ void HTMLParser::handle_in_template(HTMLToken& token)
// -> An end-of-file token
if (token.is_end_of_file()) {
// If there is no template element on the stack of open elements, then stop parsing. (fragment case)
if (!m_stack_of_open_elements.contains(HTML::TagNames::template_)) {
if (!m_stack_of_open_elements.contains_template_element()) {
VERIFY(m_parsing_fragment);
stop_parsing();
return;
Expand Down Expand Up @@ -4316,6 +4316,9 @@ void HTMLParser::reset_the_insertion_mode_appropriately()
node = m_stack_of_open_elements.elements().at(i).ptr();
}

if (node->namespace_uri() != Namespace::HTML)
continue;

if (node->local_name() == HTML::TagNames::select) {
if (!last) {
for (ssize_t j = i; j > 0; --j) {
Expand Down
11 changes: 7 additions & 4 deletions Userland/Libraries/LibWeb/HTML/Parser/StackOfOpenElements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <LibWeb/DOM/Element.h>
#include <LibWeb/HTML/Parser/HTMLParser.h>
#include <LibWeb/HTML/Parser/StackOfOpenElements.h>
#include <LibWeb/Namespace.h>

namespace Web::HTML {

Expand Down Expand Up @@ -105,18 +106,20 @@ bool StackOfOpenElements::contains(const DOM::Element& element) const
return false;
}

bool StackOfOpenElements::contains(FlyString const& tag_name) const
bool StackOfOpenElements::contains_template_element() const
{
for (auto& element_on_stack : m_elements) {
if (element_on_stack->local_name() == tag_name)
for (auto const& element : m_elements) {
if (element->namespace_uri() != Namespace::HTML)
continue;
if (element->local_name() == HTML::TagNames::template_)
return true;
}
return false;
}

void StackOfOpenElements::pop_until_an_element_with_tag_name_has_been_popped(FlyString const& tag_name)
{
while (m_elements.last()->local_name() != tag_name)
while (m_elements.last()->namespace_uri() != Namespace::HTML || m_elements.last()->local_name() != tag_name)
(void)pop();
(void)pop();
}
Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/HTML/Parser/StackOfOpenElements.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ class StackOfOpenElements {
bool has_in_scope(const DOM::Element&) const;

bool contains(const DOM::Element&) const;
bool contains(FlyString const& tag_name) const;
[[nodiscard]] bool contains_template_element() const;

auto const& elements() const { return m_elements; }
auto& elements() { return m_elements; }

void pop_until_an_element_with_tag_name_has_been_popped(FlyString const&);
void pop_until_an_element_with_tag_name_has_been_popped(FlyString const& local_name);

JS::GCPtr<DOM::Element> topmost_special_node_below(DOM::Element const&);

Expand Down

0 comments on commit ceedfb3

Please sign in to comment.