LibWeb: Account for logical property groups in set_a_declaration

When setting a declaration for a property in a logical property group,
it should appear after all other declarations which belong to the same
property group but have different mapping logic (are/aren't a logical
alias).

Gains us 1 WPT pass.

Libraries/LibWeb/CSS/CSSStyleProperties.cpp

@@ -1243,23 +1243,83 @@ bool CSSStyleProperties::set_a_css_declaration(PropertyID property_id, NonnullRe
 {
     VERIFY(!is_computed());
 
-    // FIXME: Handle logical property groups.
+    // NOTE: The below algorithm is only suggested rather than required by the spec
+    // https://drafts.csswg.org/cssom/#example-a40690cb
+    // 1. If property is a case-sensitive match for a property name of a CSS declaration in declarations, follow these substeps:
+    auto maybe_target_index = m_properties.find_first_index_if([&](auto declaration) { return declaration.property_id == property_id; });
 
-    for (auto& property : m_properties) {
-        if (property.property_id == property_id) {
-            if (property.important == important && *property.value == *value)
+    if (maybe_target_index.has_value()) {
+        // 1. Let target declaration be such CSS declaration.
+        auto target_declaration = m_properties[maybe_target_index.value()];
+
+        // 2. Let needs append be false.
+        bool needs_append = false;
+
+        auto logical_property_group_for_set_property = logical_property_group_for_property(property_id);
+
+        // NOTE: If the property of the declaration being set has no logical property group then it's not possible for
+        //       one of the later declarations to share that logical property group so we can skip checking.
+        if (logical_property_group_for_set_property.has_value()) {
+            auto set_property_is_logical_alias = property_is_logical_alias(property_id);
+
+            // 3. For each declaration in declarations after target declaration:
+            for (size_t i = maybe_target_index.value() + 1; i < m_properties.size(); ++i) {
+                // 1. If declaration’s property name is not in the same logical property group as property, then continue.
+                if (logical_property_group_for_property(m_properties[i].property_id) != logical_property_group_for_set_property)
+                    continue;
+
+                // 2. If declaration’ property name has the same mapping logic as property, then continue.
+                if (property_is_logical_alias(m_properties[i].property_id) == set_property_is_logical_alias)
+                    continue;
+
+                // 3. Let needs append be true.
+                needs_append = true;
+
+                // 4. Break.
+                break;
+            }
+        }
+
+        // 4. If needs append is false, then:
+        if (!needs_append) {
+            // 1. Let needs update be false.
+            bool needs_update = false;
+
+            // 2. If target declaration’s value is not equal to component value list, then let needs update be true.
+            if (*target_declaration.value != *value)
+                needs_update = true;
+
+            // 3. If target declaration’s important flag is not equal to whether important flag is set, then let needs update be true.
+            if (target_declaration.important != important)
+                needs_update = true;
+
+            // 4. If needs update is false, then return false.
+            if (!needs_update)
                 return false;
-            property.value = move(value);
-            property.important = important;
+
+            // 5. Set target declaration’s value to component value list.
+            m_properties[maybe_target_index.value()].value = move(value);
+
+            // 6. If important flag is set, then set target declaration’s important flag, otherwise unset it.
+            m_properties[maybe_target_index.value()].important = important;
+
+            // 7. Return true.
             return true;
         }
+
+        // 5. Otherwise, remove target declaration from declarations.
+        m_properties.remove(maybe_target_index.value());
     }
 
+    // 2. Append a new CSS declaration with property name property, value component value list, and important flag set
+    //    if important flag is set to declarations.
     m_properties.append(StyleProperty {
         .important = important,
         .property_id = property_id,
         .value = move(value),
     });
+
+    // 3. Return true
     return true;
 }
 

Tests/LibWeb/Text/expected/wpt-import/css/cssom/cssstyledeclaration-setter-logical.txt

@@ -0,0 +1,6 @@
+Harness status: OK
+
+Found 1 tests
+
+1 Pass
+Pass	newly set declaration should be after all declarations in the same logical property group but have different logical kind

Tests/LibWeb/Text/input/wpt-import/css/cssom/cssstyledeclaration-setter-logical.html

@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<title>CSSOM test: declaration block after setting via CSSOM</title>
+<link rel="help" href="https://drafts.csswg.org/cssom/#set-a-css-declaration-value">
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<div id="log"></div>
+<div id="test"></div>
+<script>
+  test(function() {
+    const PHYSICAL_PROPS = ["padding-top", "padding-right",
+                            "padding-bottom", "padding-left"];
+    const LOGICAL_PROPS = ["padding-block-start", "padding-block-end",
+                           "padding-inline-start", "padding-inline-end"];
+    let elem = document.getElementById("test");
+    let decls = new Map;
+
+    {
+      let css = []
+      let i = 0;
+      for (let name of [...PHYSICAL_PROPS, ...LOGICAL_PROPS]) {
+        let value = `${i++}px`;
+        css.push(`${name}: ${value};`);
+        decls.set(name, value);
+      }
+      elem.setAttribute("style", css.join(" "));
+    }
+
+    let style = elem.style;
+    function indexOfProperty(prop) {
+      return Array.prototype.indexOf.apply(style, [prop]);
+    }
+    function assertPropAfterProps(prop, props, msg) {
+      let index = indexOfProperty(prop);
+      for (let p of props) {
+        assert_less_than(indexOfProperty(p), index, `${prop} should be after ${p} ${msg}`);
+      }
+    }
+    // We are not changing any value in this test, just order.
+    function assertValueUnchanged() {
+      for (let [name, value] of decls.entries()) {
+        assert_equals(style.getPropertyValue(name), value,
+                      `value of ${name} shouldn't be changed`);
+      }
+    }
+
+    assertValueUnchanged();
+    // Check that logical properties are all after physical properties
+    // at the beginning.
+    for (let p of LOGICAL_PROPS) {
+      assertPropAfterProps(p, PHYSICAL_PROPS, "initially");
+    }
+
+    // Try setting a longhand.
+    style.setProperty("padding-top", "0px");
+    assertValueUnchanged();
+    // Check that padding-top is after logical properties, but other
+    // physical properties are still before logical properties.
+    assertPropAfterProps("padding-top", LOGICAL_PROPS, "after setting padding-top");
+    for (let p of LOGICAL_PROPS) {
+      assertPropAfterProps(p, PHYSICAL_PROPS.filter(prop => prop != "padding-top"),
+                           "after setting padding-top");
+    }
+
+    // Try setting a shorthand.
+    style.setProperty("padding", "0px 1px 2px 3px");
+    assertValueUnchanged();
+    // Check that all physical properties are now after logical properties.
+    for (let p of PHYSICAL_PROPS) {
+      assertPropAfterProps(p, LOGICAL_PROPS, "after setting padding shorthand");
+    }
+  }, "newly set declaration should be after all declarations " +
+  "in the same logical property group but have different logical kind");
+</script>