mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-08-29 21:57:18 +00:00
LibWeb: Treat CSS selectors containing undeclared namespaces as invalid
Selectors containing undeclared namespaces should be considered invalid, not just not matching any elements. Gains us 3 new WPT passes.
This commit is contained in:
parent
5fcf3d0b05
commit
6584ae0080
Notes:
github-actions[bot]
2025-06-24 11:52:28 +00:00
Author: https://github.com/Calme1709
Commit: 6584ae0080
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5184
Reviewed-by: https://github.com/AtkinsSJ ✅
17 changed files with 157 additions and 11 deletions
|
@ -47,7 +47,7 @@ WebIDL::ExceptionOr<u32> CSSGroupingRule::insert_rule(StringView rule, u32 index
|
||||||
{
|
{
|
||||||
// The insertRule(rule, index) method must return the result of invoking insert a CSS rule rule into the child CSS
|
// The insertRule(rule, index) method must return the result of invoking insert a CSS rule rule into the child CSS
|
||||||
// rules at index, with the nested flag set.
|
// rules at index, with the nested flag set.
|
||||||
TRY(m_rules->insert_a_css_rule(rule, index, CSSRuleList::Nested::Yes));
|
TRY(m_rules->insert_a_css_rule(rule, index, CSSRuleList::Nested::Yes, m_parent_style_sheet->declared_namespaces()));
|
||||||
|
|
||||||
// AD-HOC: The spec doesn't say where to set the parent rule, so we'll do it here.
|
// AD-HOC: The spec doesn't say where to set the parent rule, so we'll do it here.
|
||||||
m_rules->item(index)->set_parent_rule(this);
|
m_rules->item(index)->set_parent_rule(this);
|
||||||
|
|
|
@ -49,8 +49,9 @@ void CSSRuleList::visit_edges(Cell::Visitor& visitor)
|
||||||
visitor.visit(m_owner_rule);
|
visitor.visit(m_owner_rule);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// AD-HOC: The spec doesn't include a declared_namespaces parameter, but we need it to handle parsing of namespaced selectors.
|
||||||
// https://drafts.csswg.org/cssom/#insert-a-css-rule
|
// https://drafts.csswg.org/cssom/#insert-a-css-rule
|
||||||
WebIDL::ExceptionOr<unsigned> CSSRuleList::insert_a_css_rule(Variant<StringView, CSSRule*> rule, u32 index, Nested nested)
|
WebIDL::ExceptionOr<unsigned> CSSRuleList::insert_a_css_rule(Variant<StringView, CSSRule*> rule, u32 index, Nested nested, HashTable<FlyString> const& declared_namespaces)
|
||||||
{
|
{
|
||||||
// 1. Set length to the number of items in list.
|
// 1. Set length to the number of items in list.
|
||||||
auto length = m_rules.size();
|
auto length = m_rules.size();
|
||||||
|
@ -63,9 +64,13 @@ WebIDL::ExceptionOr<unsigned> CSSRuleList::insert_a_css_rule(Variant<StringView,
|
||||||
// NOTE: The insert-a-css-rule spec expects `rule` to be a string, but the CSSStyleSheet.insertRule()
|
// NOTE: The insert-a-css-rule spec expects `rule` to be a string, but the CSSStyleSheet.insertRule()
|
||||||
// spec calls this algorithm with an already-parsed CSSRule. So, we use a Variant and skip step 3
|
// spec calls this algorithm with an already-parsed CSSRule. So, we use a Variant and skip step 3
|
||||||
// if that variant holds a CSSRule already.
|
// if that variant holds a CSSRule already.
|
||||||
|
|
||||||
CSSRule* new_rule = nullptr;
|
CSSRule* new_rule = nullptr;
|
||||||
if (rule.has<StringView>()) {
|
if (rule.has<StringView>()) {
|
||||||
new_rule = parse_css_rule(Parser::ParsingParams { realm() }, rule.get<StringView>());
|
Parser::ParsingParams parsing_params { realm() };
|
||||||
|
parsing_params.declared_namespaces = declared_namespaces;
|
||||||
|
|
||||||
|
new_rule = parse_css_rule(parsing_params, rule.get<StringView>());
|
||||||
} else {
|
} else {
|
||||||
new_rule = rule.get<CSSRule*>();
|
new_rule = rule.get<CSSRule*>();
|
||||||
}
|
}
|
||||||
|
@ -74,6 +79,8 @@ WebIDL::ExceptionOr<unsigned> CSSRuleList::insert_a_css_rule(Variant<StringView,
|
||||||
if (!new_rule && nested == Nested::Yes) {
|
if (!new_rule && nested == Nested::Yes) {
|
||||||
Parser::ParsingParams parsing_params { realm() };
|
Parser::ParsingParams parsing_params { realm() };
|
||||||
parsing_params.rule_context = rule_context();
|
parsing_params.rule_context = rule_context();
|
||||||
|
parsing_params.declared_namespaces = declared_namespaces;
|
||||||
|
|
||||||
// - Set declarations to the results of performing parse a CSS declaration block, on argument rule.
|
// - Set declarations to the results of performing parse a CSS declaration block, on argument rule.
|
||||||
auto declarations = parse_css_property_declaration_block(parsing_params, rule.get<StringView>());
|
auto declarations = parse_css_property_declaration_block(parsing_params, rule.get<StringView>());
|
||||||
|
|
||||||
|
|
|
@ -57,7 +57,7 @@ public:
|
||||||
No,
|
No,
|
||||||
Yes,
|
Yes,
|
||||||
};
|
};
|
||||||
WebIDL::ExceptionOr<unsigned> insert_a_css_rule(Variant<StringView, CSSRule*>, u32 index, Nested = Nested::No);
|
WebIDL::ExceptionOr<unsigned> insert_a_css_rule(Variant<StringView, CSSRule*>, u32 index, Nested, HashTable<FlyString> const& declared_namespaces);
|
||||||
|
|
||||||
void for_each_effective_rule(TraversalOrder, Function<void(CSSRule const&)> const& callback) const;
|
void for_each_effective_rule(TraversalOrder, Function<void(CSSRule const&)> const& callback) const;
|
||||||
// Returns whether the match state of any media queries changed after evaluation.
|
// Returns whether the match state of any media queries changed after evaluation.
|
||||||
|
|
|
@ -126,12 +126,17 @@ void CSSStyleRule::set_selector_text(StringView selector_text)
|
||||||
clear_caches();
|
clear_caches();
|
||||||
|
|
||||||
// 1. Run the parse a group of selectors algorithm on the given value.
|
// 1. Run the parse a group of selectors algorithm on the given value.
|
||||||
|
Parser::ParsingParams parsing_params { realm() };
|
||||||
|
|
||||||
|
if (m_parent_style_sheet)
|
||||||
|
parsing_params.declared_namespaces = m_parent_style_sheet->declared_namespaces();
|
||||||
|
|
||||||
Optional<SelectorList> parsed_selectors;
|
Optional<SelectorList> parsed_selectors;
|
||||||
if (parent_style_rule()) {
|
if (parent_style_rule()) {
|
||||||
// AD-HOC: If we're a nested style rule, then we need to parse the selector as relative and then adapt it with implicit &s.
|
// AD-HOC: If we're a nested style rule, then we need to parse the selector as relative and then adapt it with implicit &s.
|
||||||
parsed_selectors = parse_selector_for_nested_style_rule(Parser::ParsingParams { realm() }, selector_text);
|
parsed_selectors = parse_selector_for_nested_style_rule(parsing_params, selector_text);
|
||||||
} else {
|
} else {
|
||||||
parsed_selectors = parse_selector(Parser::ParsingParams { realm() }, selector_text);
|
parsed_selectors = parse_selector(parsing_params, selector_text);
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2. If the algorithm returns a non-null value replace the associated group of selectors with the returned value.
|
// 2. If the algorithm returns a non-null value replace the associated group of selectors with the returned value.
|
||||||
|
|
|
@ -151,7 +151,7 @@ WebIDL::ExceptionOr<unsigned> CSSStyleSheet::insert_rule(StringView rule, unsign
|
||||||
return WebIDL::SyntaxError::create(realm(), "Can't insert @import rules into a constructed stylesheet."_string);
|
return WebIDL::SyntaxError::create(realm(), "Can't insert @import rules into a constructed stylesheet."_string);
|
||||||
|
|
||||||
// 6. Return the result of invoking insert a CSS rule rule in the CSS rules at index.
|
// 6. Return the result of invoking insert a CSS rule rule in the CSS rules at index.
|
||||||
auto result = m_rules->insert_a_css_rule(parsed_rule, index);
|
auto result = m_rules->insert_a_css_rule(parsed_rule, index, CSSRuleList::Nested::No, declared_namespaces());
|
||||||
|
|
||||||
if (!result.is_exception()) {
|
if (!result.is_exception()) {
|
||||||
// NOTE: The spec doesn't say where to set the parent style sheet, so we'll do it here.
|
// NOTE: The spec doesn't say where to set the parent style sheet, so we'll do it here.
|
||||||
|
@ -375,6 +375,17 @@ Optional<FlyString> CSSStyleSheet::default_namespace() const
|
||||||
return {};
|
return {};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
HashTable<FlyString> CSSStyleSheet::declared_namespaces() const
|
||||||
|
{
|
||||||
|
HashTable<FlyString> declared_namespaces;
|
||||||
|
|
||||||
|
for (auto namespace_ : m_namespace_rules.keys()) {
|
||||||
|
declared_namespaces.set(namespace_);
|
||||||
|
}
|
||||||
|
|
||||||
|
return declared_namespaces;
|
||||||
|
}
|
||||||
|
|
||||||
Optional<FlyString> CSSStyleSheet::namespace_uri(StringView namespace_prefix) const
|
Optional<FlyString> CSSStyleSheet::namespace_uri(StringView namespace_prefix) const
|
||||||
{
|
{
|
||||||
return m_namespace_rules.get(namespace_prefix)
|
return m_namespace_rules.get(namespace_prefix)
|
||||||
|
@ -444,9 +455,14 @@ bool CSSStyleSheet::has_associated_font_loader(FontLoader& font_loader) const
|
||||||
|
|
||||||
Parser::ParsingParams CSSStyleSheet::make_parsing_params() const
|
Parser::ParsingParams CSSStyleSheet::make_parsing_params() const
|
||||||
{
|
{
|
||||||
|
Parser::ParsingParams parsing_params;
|
||||||
if (auto document = owning_document())
|
if (auto document = owning_document())
|
||||||
return Parser::ParsingParams { *document };
|
parsing_params = Parser::ParsingParams { *document };
|
||||||
return Parser::ParsingParams { realm() };
|
else
|
||||||
|
parsing_params = Parser::ParsingParams { realm() };
|
||||||
|
|
||||||
|
parsing_params.declared_namespaces = declared_namespaces();
|
||||||
|
return parsing_params;
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -72,6 +72,7 @@ public:
|
||||||
|
|
||||||
Optional<FlyString> default_namespace() const;
|
Optional<FlyString> default_namespace() const;
|
||||||
GC::Ptr<CSSNamespaceRule> default_namespace_rule() const { return m_default_namespace_rule; }
|
GC::Ptr<CSSNamespaceRule> default_namespace_rule() const { return m_default_namespace_rule; }
|
||||||
|
HashTable<FlyString> declared_namespaces() const;
|
||||||
|
|
||||||
Optional<FlyString> namespace_uri(StringView namespace_prefix) const;
|
Optional<FlyString> namespace_uri(StringView namespace_prefix) const;
|
||||||
|
|
||||||
|
|
|
@ -66,6 +66,7 @@ Parser::Parser(ParsingParams const& context, Vector<Token> tokens)
|
||||||
, m_tokens(move(tokens))
|
, m_tokens(move(tokens))
|
||||||
, m_token_stream(m_tokens)
|
, m_token_stream(m_tokens)
|
||||||
, m_rule_context(move(context.rule_context))
|
, m_rule_context(move(context.rule_context))
|
||||||
|
, m_declared_namespaces(move(context.declared_namespaces))
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -141,6 +142,8 @@ GC::RootVector<GC::Ref<CSSRule>> Parser::convert_rules(Vector<Rule> const& raw_r
|
||||||
|
|
||||||
if (!namespace_rules_valid)
|
if (!namespace_rules_valid)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
|
m_declared_namespaces.set(as<CSSNamespaceRule>(*rule).prefix());
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
import_rules_valid = false;
|
import_rules_valid = false;
|
||||||
|
|
|
@ -81,6 +81,7 @@ struct ParsingParams {
|
||||||
ParsingMode mode { ParsingMode::Normal };
|
ParsingMode mode { ParsingMode::Normal };
|
||||||
|
|
||||||
Vector<RuleContext> rule_context;
|
Vector<RuleContext> rule_context;
|
||||||
|
HashTable<FlyString> declared_namespaces;
|
||||||
};
|
};
|
||||||
|
|
||||||
// The very large CSS Parser implementation code is broken up among several .cpp files:
|
// The very large CSS Parser implementation code is broken up among several .cpp files:
|
||||||
|
@ -543,6 +544,7 @@ private:
|
||||||
bool context_allows_quirky_length() const;
|
bool context_allows_quirky_length() const;
|
||||||
|
|
||||||
Vector<RuleContext> m_rule_context;
|
Vector<RuleContext> m_rule_context;
|
||||||
|
HashTable<FlyString> m_declared_namespaces;
|
||||||
|
|
||||||
Vector<PseudoClass> m_pseudo_class_context; // Stack of pseudo-class functions we're currently inside
|
Vector<PseudoClass> m_pseudo_class_context; // Stack of pseudo-class functions we're currently inside
|
||||||
};
|
};
|
||||||
|
|
|
@ -258,8 +258,10 @@ Optional<Selector::SimpleSelector::QualifiedName> Parser::parse_selector_qualifi
|
||||||
? Selector::SimpleSelector::QualifiedName::NamespaceType::Any
|
? Selector::SimpleSelector::QualifiedName::NamespaceType::Any
|
||||||
: Selector::SimpleSelector::QualifiedName::NamespaceType::Named;
|
: Selector::SimpleSelector::QualifiedName::NamespaceType::Named;
|
||||||
|
|
||||||
// FIXME: https://www.w3.org/TR/selectors-4/#invalid
|
// https://www.w3.org/TR/selectors-4/#invalid
|
||||||
// - a simple selector containing an undeclared namespace prefix is invalid
|
// a simple selector containing an undeclared namespace prefix is invalid
|
||||||
|
if (namespace_type == Selector::SimpleSelector::QualifiedName::NamespaceType::Named && !m_declared_namespaces.contains(namespace_))
|
||||||
|
return {};
|
||||||
|
|
||||||
transaction.commit();
|
transaction.commit();
|
||||||
return Selector::SimpleSelector::QualifiedName {
|
return Selector::SimpleSelector::QualifiedName {
|
||||||
|
|
|
@ -0,0 +1,10 @@
|
||||||
|
<html xmlns="http://www.w3.org/1999/xhtml">
|
||||||
|
<head>
|
||||||
|
<link rel="author" title="L. David Baron" href="https://dbaron.org/"/>
|
||||||
|
<link rel="author" title="Mozilla" href="http://mozilla.org/"/>
|
||||||
|
<title>CSS Namespaces Test Suite reference</title>
|
||||||
|
</head>
|
||||||
|
<body>
|
||||||
|
<p><test style="background: lime">This sentence should have a green background.</test></p>
|
||||||
|
</body>
|
||||||
|
</html>
|
|
@ -0,0 +1,38 @@
|
||||||
|
<!doctype html>
|
||||||
|
<head>
|
||||||
|
<title>CSS Test (Conditional Rules): Invalid rules after @supports</title>
|
||||||
|
<!-- This test is paired with at-media-003.html ; please keep them in sync -->
|
||||||
|
<link rel="author" title="Elika J. Etemad" href="http://fantasai.inkedblade.net/contact">
|
||||||
|
<link rel="help" href="https://www.w3.org/TR/css-conditional-3/#contents-of">
|
||||||
|
<link rel="match" href="../../../../expected/wpt-import/css/css-conditional/at-supports-001-ref.html">
|
||||||
|
<meta name="assert"
|
||||||
|
content="Test passes if rules required to occur before all style rules are invalid after @supports.">
|
||||||
|
<style>
|
||||||
|
@namespace x "http://www.w3.org/";
|
||||||
|
@supports (background: blue) { /* invalidates later rules even if empty */ }
|
||||||
|
@import "support/fail.css";
|
||||||
|
@namespace y "http://www.w3.org/";
|
||||||
|
|
||||||
|
.test1, x|div { background: green; }
|
||||||
|
.test1, y|div { background: red; }
|
||||||
|
|
||||||
|
div {
|
||||||
|
background: red;
|
||||||
|
height: 50px;
|
||||||
|
width: 100px;
|
||||||
|
}
|
||||||
|
</style>
|
||||||
|
<style>
|
||||||
|
@supports (background: blue) {
|
||||||
|
/* @supports isn't invalidated, only misordered stuff after it is */
|
||||||
|
.test2 { background: green; }
|
||||||
|
}
|
||||||
|
@import "support/fail.css";
|
||||||
|
</style>
|
||||||
|
</head>
|
||||||
|
<body>
|
||||||
|
<p>Test passes if there is a <strong>filled green square</strong> and <strong>no red</strong>.</p>
|
||||||
|
<div class="test1"></div>
|
||||||
|
<div class="test2"></div>
|
||||||
|
</body>
|
||||||
|
</html>
|
|
@ -0,0 +1 @@
|
||||||
|
* { background:red ! important }
|
|
@ -0,0 +1,21 @@
|
||||||
|
<html xmlns="http://www.w3.org/1999/xhtml">
|
||||||
|
<head>
|
||||||
|
<link rel="author" title="Anne van Kesteren" href="http://annevankesteren.nl/"/>
|
||||||
|
<link rel="author" title="Opera Software ASA" href="http://opera.com/"/>
|
||||||
|
<link rel="help" href="http://www.w3.org/TR/css-namespaces-3/#scope"/>
|
||||||
|
<link rel="match" href="../../../../expected/wpt-import/css/css-namespaces/reference/ref-lime-1.xml"/>
|
||||||
|
<title>CSS Namespaces Test Suite: scope @import</title>
|
||||||
|
<style>
|
||||||
|
test { background:lime }
|
||||||
|
</style>
|
||||||
|
<style>
|
||||||
|
@import url("support/scope-002a.css");
|
||||||
|
@import url("support/scope-002b.css");
|
||||||
|
@namespace w "test";
|
||||||
|
x|test { background:red }
|
||||||
|
</style>
|
||||||
|
</head>
|
||||||
|
<body>
|
||||||
|
<p><test xmlns="test">This sentence should have a green background.</test></p>
|
||||||
|
</body>
|
||||||
|
</html>
|
|
@ -0,0 +1,3 @@
|
||||||
|
@namespace x url("test");
|
||||||
|
@namespace y url("test");
|
||||||
|
w|test { background:red }
|
|
@ -0,0 +1,2 @@
|
||||||
|
y|test { background: red }
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
Harness status: OK
|
||||||
|
|
||||||
|
Found 1 tests
|
||||||
|
|
||||||
|
1 Pass
|
||||||
|
Pass CSS Test: @namespace in CSSOM is not severely broken
|
|
@ -0,0 +1,29 @@
|
||||||
|
<!doctype html>
|
||||||
|
<title>CSS Test: @namespace in CSSOM is not severely broken</title>
|
||||||
|
<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
|
||||||
|
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1464865">
|
||||||
|
<link rel="help" href="https://drafts.csswg.org/cssom/#insert-a-css-rule">
|
||||||
|
<script src="../../resources/testharness.js"></script>
|
||||||
|
<script src="../../resources/testharnessreport.js"></script>
|
||||||
|
<style id="s">
|
||||||
|
div { color: green }
|
||||||
|
</style>
|
||||||
|
<div>Should be green</div>
|
||||||
|
<script>
|
||||||
|
test(function() {
|
||||||
|
assert_throws_dom("InvalidStateError", function() {
|
||||||
|
s.sheet.insertRule('@namespace myhtml url("http://www.w3.org/1999/xhtml")', 0);
|
||||||
|
});
|
||||||
|
assert_equals(s.sheet.cssRules.length, 1, "Shouldn't have been inserted");
|
||||||
|
assert_throws_dom("SyntaxError", function() {
|
||||||
|
s.sheet.insertRule("myhtml|div { color: red !important }", 0);
|
||||||
|
});
|
||||||
|
assert_equals(s.sheet.cssRules.length, 1);
|
||||||
|
assert_equals(
|
||||||
|
getComputedStyle(document.querySelector("div")).color,
|
||||||
|
"rgb(0, 128, 0)",
|
||||||
|
"Namespace shouldn't be registered"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
</script>
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue