diff --git a/Libraries/LibWeb/CSS/CSSDescriptors.cpp b/Libraries/LibWeb/CSS/CSSDescriptors.cpp index 1d77eaaf020..fcadfc28801 100644 --- a/Libraries/LibWeb/CSS/CSSDescriptors.cpp +++ b/Libraries/LibWeb/CSS/CSSDescriptors.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include namespace Web::CSS { @@ -95,12 +96,27 @@ WebIDL::ExceptionOr CSSDescriptors::set_property(StringView property, Stri // 7. Let updated be false. auto updated = false; - // 8. If property is a shorthand property... - // NB: Descriptors can't be shorthands. + // 8. If property is a shorthand property, then for each longhand property longhand that property maps to, in canonical order, follow these substeps: + if (is_shorthand(m_at_rule_id, *descriptor_id)) { + for_each_expanded_longhand(m_at_rule_id, *descriptor_id, component_value_list, [this, &updated, priority](DescriptorID longhand_id, auto longhand_value) { + VERIFY(longhand_value); + + // 1. Let longhand result be the result of set the CSS declaration longhand with the appropriate value(s) + // from component value list, with the important flag set if priority is not the empty string, and unset + // otherwise, and with the list of declarations being the declarations. + auto longhand_result = set_a_css_declaration(longhand_id, longhand_value.release_nonnull(), priority.is_empty() ? Important::No : Important::Yes); + + // 2. If longhand result is true, let updated be true. + if (longhand_result) + updated = true; + }); + } // 9. Otherwise, let updated be the result of set the CSS declaration property with value component value list, // with the important flag set if priority is not the empty string, and unset otherwise, and with the list of // declarations being the declarations. - updated = set_a_css_declaration(*descriptor_id, *component_value_list, !priority.is_empty() ? Important::Yes : Important::No); + else { + updated = set_a_css_declaration(*descriptor_id, *component_value_list, !priority.is_empty() ? Important::Yes : Important::No); + } // 10. If updated is true, update style attribute for the CSS declaration block. if (updated) @@ -124,14 +140,21 @@ WebIDL::ExceptionOr CSSDescriptors::remove_property(StringView property) // 4. Let removed be false. bool removed = false; + auto descriptor_id = descriptor_id_from_string(m_at_rule_id, property); - // 5. If property is a shorthand property... - // NB: Descriptors can't be shorthands. + // 5. If property is a shorthand property, for each longhand property longhand that property maps to: + if (descriptor_id.has_value() && is_shorthand(m_at_rule_id, *descriptor_id)) { + for_each_expanded_longhand(m_at_rule_id, *descriptor_id, nullptr, [this, &removed](DescriptorID longhand_id, auto const&) { + // 1. If longhand is not a property name of a CSS declaration in the declarations, continue. + // 2. Remove that CSS declaration and let removed be true. + if (m_descriptors.remove_first_matching([longhand_id](auto& entry) { return entry.descriptor_id == longhand_id; })) { + removed = true; + } + }); + } // 6. Otherwise, if property is a case-sensitive match for a property name of a CSS declaration in the // declarations, remove that CSS declaration and let removed be true. - // auto descriptor_id = descriptor_from_string() - auto descriptor_id = descriptor_id_from_string(m_at_rule_id, property); - if (descriptor_id.has_value()) { + else if (descriptor_id.has_value()) { removed = m_descriptors.remove_first_matching([descriptor_id](auto& entry) { return entry.descriptor_id == *descriptor_id; }); } @@ -256,4 +279,56 @@ RefPtr CSSDescriptors::descriptor_or_initial_value(Descript return descriptor_initial_value(m_at_rule_id, descriptor_id); } +bool is_shorthand(AtRuleID at_rule, DescriptorID descriptor) +{ + if (at_rule == AtRuleID::Page && descriptor == DescriptorID::Margin) + return true; + + return false; +} + +void for_each_expanded_longhand(AtRuleID at_rule, DescriptorID descriptor, RefPtr value, Function)> callback) +{ + if (at_rule == AtRuleID::Page && descriptor == DescriptorID::Margin) { + if (!value) { + callback(DescriptorID::MarginTop, nullptr); + callback(DescriptorID::MarginRight, nullptr); + callback(DescriptorID::MarginBottom, nullptr); + callback(DescriptorID::MarginLeft, nullptr); + return; + } + + if (value->is_value_list()) { + auto& values = value->as_value_list().values(); + if (values.size() == 4) { + callback(DescriptorID::MarginTop, values[0]); + callback(DescriptorID::MarginRight, values[1]); + callback(DescriptorID::MarginBottom, values[2]); + callback(DescriptorID::MarginLeft, values[3]); + } else if (values.size() == 3) { + callback(DescriptorID::MarginTop, values[0]); + callback(DescriptorID::MarginRight, values[1]); + callback(DescriptorID::MarginBottom, values[2]); + callback(DescriptorID::MarginLeft, values[1]); + } else if (values.size() == 2) { + callback(DescriptorID::MarginTop, values[0]); + callback(DescriptorID::MarginRight, values[1]); + callback(DescriptorID::MarginBottom, values[0]); + callback(DescriptorID::MarginLeft, values[1]); + } else if (values.size() == 1) { + callback(DescriptorID::MarginTop, values[0]); + callback(DescriptorID::MarginRight, values[0]); + callback(DescriptorID::MarginBottom, values[0]); + callback(DescriptorID::MarginLeft, values[0]); + } + + } else { + callback(DescriptorID::MarginTop, *value); + callback(DescriptorID::MarginRight, *value); + callback(DescriptorID::MarginBottom, *value); + callback(DescriptorID::MarginLeft, *value); + } + } +} + } diff --git a/Libraries/LibWeb/CSS/CSSDescriptors.h b/Libraries/LibWeb/CSS/CSSDescriptors.h index bb9336d70b3..64cb0fd7a81 100644 --- a/Libraries/LibWeb/CSS/CSSDescriptors.h +++ b/Libraries/LibWeb/CSS/CSSDescriptors.h @@ -45,4 +45,7 @@ private: Vector m_descriptors; }; +bool is_shorthand(AtRuleID, DescriptorID); +void for_each_expanded_longhand(AtRuleID, DescriptorID, RefPtr, Function)>); + } diff --git a/Libraries/LibWeb/CSS/Parser/RuleParsing.cpp b/Libraries/LibWeb/CSS/Parser/RuleParsing.cpp index ebf6e4f0370..d9a48367116 100644 --- a/Libraries/LibWeb/CSS/Parser/RuleParsing.cpp +++ b/Libraries/LibWeb/CSS/Parser/RuleParsing.cpp @@ -40,6 +40,49 @@ namespace Web::CSS::Parser { +// A helper that ensures only the last instance of each descriptor is included, while also handling shorthands. +class DescriptorList { +public: + DescriptorList(AtRuleID at_rule) + : m_at_rule(at_rule) + { + } + + void append(Descriptor&& descriptor) + { + if (is_shorthand(m_at_rule, descriptor.descriptor_id)) { + for_each_expanded_longhand(m_at_rule, descriptor.descriptor_id, descriptor.value, [this](auto longhand_id, auto longhand_value) { + append_internal(Descriptor { longhand_id, longhand_value.release_nonnull() }); + }); + return; + } + + append_internal(move(descriptor)); + } + + Vector release_descriptors() + { + return move(m_descriptors); + } + +private: + void append_internal(Descriptor&& descriptor) + { + if (m_seen_descriptor_ids.contains(descriptor.descriptor_id)) { + m_descriptors.remove_first_matching([&descriptor](Descriptor const& existing) { + return existing.descriptor_id == descriptor.descriptor_id; + }); + } else { + m_seen_descriptor_ids.set(descriptor.descriptor_id); + } + m_descriptors.append(move(descriptor)); + } + + AtRuleID m_at_rule; + Vector m_descriptors; + HashTable m_seen_descriptor_ids; +}; + GC::Ptr Parser::convert_to_rule(Rule const& rule, Nested nested) { return rule.visit( @@ -582,22 +625,14 @@ GC::Ptr Parser::convert_to_property_rule(AtRule const& rule) GC::Ptr Parser::convert_to_font_face_rule(AtRule const& rule) { // https://drafts.csswg.org/css-fonts/#font-face-rule - Vector descriptors; - HashTable seen_descriptor_ids; + DescriptorList descriptors { AtRuleID::FontFace }; rule.for_each_as_declaration_list([&](auto& declaration) { if (auto descriptor = convert_to_descriptor(AtRuleID::FontFace, declaration); descriptor.has_value()) { - if (seen_descriptor_ids.contains(descriptor->descriptor_id)) { - descriptors.remove_first_matching([&descriptor](Descriptor const& existing) { - return existing.descriptor_id == descriptor->descriptor_id; - }); - } else { - seen_descriptor_ids.set(descriptor->descriptor_id); - } descriptors.append(descriptor.release_value()); } }); - return CSSFontFaceRule::create(realm(), CSSFontFaceDescriptors::create(realm(), move(descriptors))); + return CSSFontFaceRule::create(realm(), CSSFontFaceDescriptors::create(realm(), descriptors.release_descriptors())); } static Optional parse_page_selector_list(Vector const& component_values) @@ -671,8 +706,7 @@ GC::Ptr Parser::convert_to_page_rule(AtRule const& rule) return nullptr; GC::RootVector> child_rules { realm().heap() }; - Vector descriptors; - HashTable seen_descriptor_ids; + DescriptorList descriptors { AtRuleID::Page }; rule.for_each_as_declaration_rule_list( [&](auto& at_rule) { // FIXME: Parse margin rules here. @@ -680,19 +714,12 @@ GC::Ptr Parser::convert_to_page_rule(AtRule const& rule) }, [&](auto& declaration) { if (auto descriptor = convert_to_descriptor(AtRuleID::Page, declaration); descriptor.has_value()) { - if (seen_descriptor_ids.contains(descriptor->descriptor_id)) { - descriptors.remove_first_matching([&descriptor](Descriptor const& existing) { - return existing.descriptor_id == descriptor->descriptor_id; - }); - } else { - seen_descriptor_ids.set(descriptor->descriptor_id); - } descriptors.append(descriptor.release_value()); } }); auto rule_list = CSSRuleList::create(realm(), child_rules); - return CSSPageRule::create(realm(), page_selectors.release_value(), CSSPageDescriptors::create(realm(), move(descriptors)), rule_list); + return CSSPageRule::create(realm(), page_selectors.release_value(), CSSPageDescriptors::create(realm(), descriptors.release_descriptors()), rule_list); } } diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-001.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-001.txt index e1adf5c44c5..8448095aced 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-001.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-001.txt @@ -2,9 +2,8 @@ Harness status: OK Found 4 tests -3 Pass -1 Fail +4 Pass Pass There should be 3 @page rules. Pass Rule #0 Pass Rule #1 -Fail Rule #2 \ No newline at end of file +Pass Rule #2 \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-002.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-002.txt index 4d99763c61f..eef35303998 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-002.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-page/cssom/page-002.txt @@ -2,6 +2,6 @@ Harness status: OK Found 2 tests -2 Fail -Fail Add declarations -Fail Remove declarations \ No newline at end of file +2 Pass +Pass Add declarations +Pass Remove declarations \ No newline at end of file