From 1b72766e4eeffa6d19b77e9c7c7bd6cff2951bb8 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 28 Jul 2021 12:34:05 +0100 Subject: [PATCH] LibWeb: Fix issues with CSS attribute selector handling This is three small, related changes: 1. Element::has_attribute() now returns true if the attribute exists but has no value. (eg, `
` -> `has_attribute("foo")`) 2. SelectorEngine::matches_attribute() now makes sure there is a first segment before comparing it, fixing a crash. 3. CSS::Parser now converts attribute names in attribute selectors to lowercase, to match the expectations of the rest of the system. Converting to lowercase is not always correct, depending on language, but since we only currently support HTML, and that expects them to be case-insensitive, it is fine for now. --- Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp | 7 ++++++- Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp | 6 ++++-- Userland/Libraries/LibWeb/DOM/Element.h | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 6d787eeaef..0a00a88a7b 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -416,7 +416,12 @@ Result Parser::parse_si .type = Selector::SimpleSelector::Type::Attribute, .attribute = { .match_type = Selector::SimpleSelector::Attribute::MatchType::HasAttribute, - .name = attribute_part.token().ident(), + // FIXME: Case-sensitivity is defined by the document language. + // HTML is insensitive with attribute names, and our code generally assumes + // they are converted to lowercase, so we do that here too. If we want to be + // correct with XML later, we'll need to keep the original case and then do + // a case-insensitive compare later. + .name = attribute_part.token().ident().to_lowercase_string(), } }; diff --git a/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp b/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp index 026dee981b..d614a3d3d4 100644 --- a/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp +++ b/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp @@ -40,9 +40,11 @@ static bool matches_attribute(CSS::Selector::SimpleSelector::Attribute const& at case CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsString: return element.attribute(attribute.name).contains(attribute.value); break; - case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithSegment: - return element.attribute(attribute.name).split_view('-').first() == attribute.value; + case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithSegment: { + auto segments = element.attribute(attribute.name).split_view('-'); + return !segments.is_empty() && segments.first() == attribute.value; break; + } case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithString: return element.attribute(attribute.name).starts_with(attribute.value); break; diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h index c4b360f84e..3878cc433d 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.h +++ b/Userland/Libraries/LibWeb/DOM/Element.h @@ -45,7 +45,7 @@ public: // NOTE: This is for the JS bindings const FlyString& namespace_uri() const { return namespace_(); } - bool has_attribute(const FlyString& name) const { return !attribute(name).is_null(); } + bool has_attribute(const FlyString& name) const { return find_attribute(name) != nullptr; } bool has_attributes() const { return !m_attributes.is_empty(); } String attribute(const FlyString& name) const; String get_attribute(const FlyString& name) const { return attribute(name); }