From 43081d97c792cd5aea65cbd1657cd578f4d2237e Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 20 Nov 2024 17:28:23 +0100 Subject: [PATCH 1/5] LibWeb: Update property_id_from_string() generator to handle ::Custom --- .../Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp index fbdb962be88..b909d12bf6c 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSPropertyID.cpp @@ -402,6 +402,7 @@ ErrorOr generate_implementation_file(JsonObject& properties, Core::File& f #include #include #include +#include #include #include #include @@ -436,6 +437,9 @@ Optional property_id_from_camel_case_string(StringView string) Optional property_id_from_string(StringView string) { + if (is_a_custom_property_name_string(string)) + return PropertyID::Custom; + if (Infra::is_ascii_case_insensitive_match(string, "all"sv)) return PropertyID::All; )~~~"); From a2cdef27c9f5cbf3dd73a5437e6ae8015c3b364a Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 20 Nov 2024 15:48:04 +0100 Subject: [PATCH 2/5] LibWeb: Allow custom properties in CSSStyleDeclaration.setProperty() This change fixes unhoverable toolbar on https://excalidraw.com/ The problem was that React.js uses setProperty() to add style properties specified in the "style" attribute in the virtual DOM, and we were failing to add the CSS variable used to set the "pointer-events" value to "all". --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 34 +++++++++++++------ Libraries/LibWeb/CSS/CSSStyleDeclaration.h | 6 ++-- .../CSSStyleDeclaration-custom-properties.txt | 1 + ...CSSStyleDeclaration-custom-properties.html | 19 +++++++++++ 4 files changed, 47 insertions(+), 13 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt create mode 100644 Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 56315c7aa4a..c95524c7d6d 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -102,8 +102,13 @@ Optional PropertyOwningCSSStyleDeclaration::property(PropertyID p } // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty -WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(PropertyID property_id, StringView value, StringView priority) +WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(StringView property_name, StringView value, StringView priority) { + auto maybe_property_id = property_id_from_string(property_name); + if (!maybe_property_id.has_value()) + return {}; + auto property_id = maybe_property_id.value(); + // 1. If the computed flag is set, then throw a NoModificationAllowedError exception. // NOTE: This is handled by the virtual override in ResolvedCSSStyleDeclaration. @@ -146,10 +151,22 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(Proper } // 9. Otherwise, else { - // 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(property_id, *component_value_list, !priority.is_empty() ? Important::Yes : Important::No); + if (property_id == PropertyID::Custom) { + auto custom_name = FlyString::from_utf8_without_validation(property_name.bytes()); + StyleProperty style_property { + .important = !priority.is_empty() ? Important::Yes : Important::No, + .property_id = property_id, + .value = component_value_list.release_nonnull(), + .custom_name = custom_name, + }; + m_custom_properties.set(custom_name, style_property); + updated = true; + } else { + // 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(property_id, *component_value_list, !priority.is_empty() ? Important::Yes : Important::No); + } } // 10. If updated is true, update style attribute for the CSS declaration block. @@ -291,12 +308,9 @@ StringView CSSStyleDeclaration::get_property_priority(StringView property_name) return maybe_property->important == Important::Yes ? "important"sv : ""sv; } -WebIDL::ExceptionOr CSSStyleDeclaration::set_property(StringView property_name, StringView css_text, StringView priority) +WebIDL::ExceptionOr CSSStyleDeclaration::set_property(PropertyID property_id, StringView css_text, StringView priority) { - auto property_id = property_id_from_string(property_name); - if (!property_id.has_value()) - return {}; - return set_property(property_id.value(), css_text, priority); + return set_property(string_from_property_id(property_id), css_text, priority); } WebIDL::ExceptionOr CSSStyleDeclaration::remove_property(StringView property_name) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index 8fb18c23f06..be01254ad40 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -30,10 +30,10 @@ public: virtual Optional property(PropertyID) const = 0; - virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority = ""sv) = 0; + virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority = ""sv); virtual WebIDL::ExceptionOr remove_property(PropertyID) = 0; - virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority); + virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) = 0; virtual WebIDL::ExceptionOr remove_property(StringView property_name); String get_property_value(StringView property) const; @@ -76,7 +76,7 @@ public: virtual Optional property(PropertyID) const override; - virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority) override; + virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr remove_property(PropertyID) override; Vector const& properties() const { return m_properties; } diff --git a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt new file mode 100644 index 00000000000..1e04124a3a2 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt @@ -0,0 +1 @@ +rgb(255, 0, 0) diff --git a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html new file mode 100644 index 00000000000..51ba809f88d --- /dev/null +++ b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html @@ -0,0 +1,19 @@ + + + + \ No newline at end of file From e026d9eb869800c873c8de6834d973459b5420c8 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 20 Nov 2024 17:21:22 +0100 Subject: [PATCH 3/5] LibWeb: Allow custom properties in CSSStyleDeclaration.getPropertyValue --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 8 ++++++++ Libraries/LibWeb/CSS/CSSStyleDeclaration.h | 4 +++- Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp | 6 ++++++ Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h | 1 + .../css/CSSStyleDeclaration-custom-properties.txt | 1 + .../input/css/CSSStyleDeclaration-custom-properties.html | 1 + 6 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index c95524c7d6d..1fe76a2751d 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -258,6 +258,14 @@ String CSSStyleDeclaration::get_property_value(StringView property_name) const if (!property_id.has_value()) return {}; + if (property_id.value() == PropertyID::Custom) { + auto maybe_custom_property = custom_property(FlyString::from_utf8_without_validation(property_name.bytes())); + if (maybe_custom_property.has_value()) { + return maybe_custom_property.value().value->to_string(); + } + return {}; + } + // 2. If property is a shorthand property, then follow these substeps: if (property_is_shorthand(property_id.value())) { // 1. Let list be a new empty array. diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index be01254ad40..f94660dcdf5 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -29,6 +29,7 @@ public: virtual String item(size_t index) const = 0; virtual Optional property(PropertyID) const = 0; + virtual Optional custom_property(FlyString const& custom_property_name) const = 0; virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority = ""sv); virtual WebIDL::ExceptionOr remove_property(PropertyID) = 0; @@ -75,13 +76,14 @@ public: virtual String item(size_t index) const override; virtual Optional property(PropertyID) const override; + virtual Optional custom_property(FlyString const& custom_property_name) const override { return m_custom_properties.get(custom_property_name); } virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr remove_property(PropertyID) override; Vector const& properties() const { return m_properties; } HashMap const& custom_properties() const { return m_custom_properties; } - Optional custom_property(FlyString const& custom_property_name) const { return m_custom_properties.get(custom_property_name); } + size_t custom_property_count() const { return m_custom_properties.size(); } virtual String serialized() const final override; diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp index ac95dc85adf..3f42a2e5360 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp @@ -601,6 +601,12 @@ Optional ResolvedCSSStyleDeclaration::property(PropertyID propert }; } +Optional ResolvedCSSStyleDeclaration::custom_property(FlyString const&) const +{ + dbgln("FIXME: ResolvedCSSStyleDeclaration::custom_property is not implemented"); + return {}; +} + static WebIDL::ExceptionOr cannot_modify_computed_property_error(JS::Realm& realm) { return WebIDL::NoModificationAllowedError::create(realm, "Cannot modify properties in result of getComputedStyle()"_string); diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h index 9a663016ed7..acc3c3b00bc 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h @@ -24,6 +24,7 @@ public: virtual String item(size_t index) const override; virtual Optional property(PropertyID) const override; + virtual Optional custom_property(FlyString const& custom_property_name) const override; virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr remove_property(PropertyID) override; diff --git a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt index 1e04124a3a2..c8d606fffe0 100644 --- a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt +++ b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt @@ -1 +1,2 @@ +style.getPropertyValue(--redcolor)=red rgb(255, 0, 0) diff --git a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html index 51ba809f88d..2c4942ddbd6 100644 --- a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html +++ b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html @@ -5,6 +5,7 @@ test(() => { const div = document.createElement('div'); div.style.setProperty("--redcolor", "red"); + println(`style.getPropertyValue(--redcolor)=${div.style.getPropertyValue("--redcolor")}`); const nested = document.createElement('div'); nested.style["backgroundColor"] = "var(--redcolor)"; From e66cdba61e42014a1e092c9983785e40cc6c7691 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 20 Nov 2024 17:48:35 +0100 Subject: [PATCH 4/5] LibWeb: Allow custom properties in CSSStyleDeclaration.removeProperty() --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 25 +++++++++++-------- Libraries/LibWeb/CSS/CSSStyleDeclaration.h | 7 +++--- .../CSSStyleDeclaration-custom-properties.txt | 1 + ...CSSStyleDeclaration-custom-properties.html | 3 +++ 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 1fe76a2751d..82dbd566eae 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -119,7 +119,7 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(String // 3. If value is the empty string, invoke removeProperty() with property as argument and return. if (value.is_empty()) { - MUST(remove_property(property_id)); + MUST(remove_property(property_name)); return {}; } @@ -177,8 +177,12 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(String } // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-removeproperty -WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(PropertyID property_id) +WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(StringView property_name) { + auto property_id = property_id_from_string(property_name); + if (!property_id.has_value()) + return String {}; + // 1. If the computed flag is set, then throw a NoModificationAllowedError exception. // NOTE: This is handled by the virtual override in ResolvedCSSStyleDeclaration. @@ -186,8 +190,7 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(P // NOTE: We've already converted it to a PropertyID enum value. // 3. Let value be the return value of invoking getPropertyValue() with property as argument. - // FIXME: The trip through string_from_property_id() here is silly. - auto value = get_property_value(string_from_property_id(property_id)); + auto value = get_property_value(property_name); // 4. Let removed be false. bool removed = false; @@ -197,7 +200,12 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(P // 2. Remove that CSS declaration and let removed be 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. - removed = m_properties.remove_first_matching([&](auto& entry) { return entry.property_id == property_id; }); + if (property_id == PropertyID::Custom) { + auto custom_name = FlyString::from_utf8_without_validation(property_name.bytes()); + removed = m_custom_properties.remove(custom_name); + } else { + removed = m_properties.remove_first_matching([&](auto& entry) { return entry.property_id == property_id; }); + } // 7. If removed is true, Update style attribute for the CSS declaration block. if (removed) @@ -321,12 +329,9 @@ WebIDL::ExceptionOr CSSStyleDeclaration::set_property(PropertyID property_ return set_property(string_from_property_id(property_id), css_text, priority); } -WebIDL::ExceptionOr CSSStyleDeclaration::remove_property(StringView property_name) +WebIDL::ExceptionOr CSSStyleDeclaration::remove_property(PropertyID property_name) { - auto property_id = property_id_from_string(property_name); - if (!property_id.has_value()) - return String {}; - return remove_property(property_id.value()); + return remove_property(string_from_property_id(property_name)); } // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index f94660dcdf5..32a898d50dc 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -32,10 +32,10 @@ public: virtual Optional custom_property(FlyString const& custom_property_name) const = 0; virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority = ""sv); - virtual WebIDL::ExceptionOr remove_property(PropertyID) = 0; + virtual WebIDL::ExceptionOr remove_property(PropertyID); virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) = 0; - virtual WebIDL::ExceptionOr remove_property(StringView property_name); + virtual WebIDL::ExceptionOr remove_property(StringView property_name) = 0; String get_property_value(StringView property) const; StringView get_property_priority(StringView property) const; @@ -79,8 +79,7 @@ public: virtual Optional custom_property(FlyString const& custom_property_name) const override { return m_custom_properties.get(custom_property_name); } virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; - virtual WebIDL::ExceptionOr remove_property(PropertyID) override; - + virtual WebIDL::ExceptionOr remove_property(StringView property_name) override; Vector const& properties() const { return m_properties; } HashMap const& custom_properties() const { return m_custom_properties; } diff --git a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt index c8d606fffe0..dde24163aa6 100644 --- a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt +++ b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt @@ -1,2 +1,3 @@ style.getPropertyValue(--redcolor)=red rgb(255, 0, 0) +rgba(0, 0, 0, 0) diff --git a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html index 2c4942ddbd6..929250da074 100644 --- a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html +++ b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html @@ -16,5 +16,8 @@ document.body.appendChild(div); println(getComputedStyle(nested).backgroundColor); + + div.style.removeProperty("--redcolor"); + println(getComputedStyle(nested).backgroundColor); }); \ No newline at end of file From 3e1b67a2a544134558350a8a8e841c8a273abd13 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 20 Nov 2024 18:11:47 +0100 Subject: [PATCH 5/5] LibWeb: Allow custom properties in getPropertyPriority() --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 6 ++++++ .../expected/css/CSSStyleDeclaration-custom-properties.txt | 1 + .../input/css/CSSStyleDeclaration-custom-properties.html | 1 + 3 files changed, 8 insertions(+) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 82dbd566eae..fef865c27f3 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -318,6 +318,12 @@ StringView CSSStyleDeclaration::get_property_priority(StringView property_name) auto property_id = property_id_from_string(property_name); if (!property_id.has_value()) return {}; + if (property_id.value() == PropertyID::Custom) { + auto maybe_custom_property = custom_property(FlyString::from_utf8_without_validation(property_name.bytes())); + if (!maybe_custom_property.has_value()) + return {}; + return maybe_custom_property.value().important == Important::Yes ? "important"sv : ""sv; + } auto maybe_property = property(property_id.value()); if (!maybe_property.has_value()) return {}; diff --git a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt index dde24163aa6..2c1e1baa910 100644 --- a/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt +++ b/Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties.txt @@ -1,3 +1,4 @@ style.getPropertyValue(--redcolor)=red +style.getPropertyPriority(--redcolor)= rgb(255, 0, 0) rgba(0, 0, 0, 0) diff --git a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html index 929250da074..178d731bc29 100644 --- a/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html +++ b/Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties.html @@ -6,6 +6,7 @@ const div = document.createElement('div'); div.style.setProperty("--redcolor", "red"); println(`style.getPropertyValue(--redcolor)=${div.style.getPropertyValue("--redcolor")}`); + println(`style.getPropertyPriority(--redcolor)=${div.style.getPropertyPriority("--redcolor")}`); const nested = document.createElement('div'); nested.style["backgroundColor"] = "var(--redcolor)";