diff --git a/dom/smil/SMILCSSValueType.cpp b/dom/smil/SMILCSSValueType.cpp index b6e786751987..f25e42fb7100 100644 --- a/dom/smil/SMILCSSValueType.cpp +++ b/dom/smil/SMILCSSValueType.cpp @@ -485,7 +485,8 @@ bool SMILCSSValueType::SetPropertyValues(NonCustomCSSPropertyId aPropertyId, "Unexpected SMIL value type"); const ValueWrapper* wrapper = ExtractValueWrapper(aValue); if (!wrapper) { - return Servo_DeclarationBlock_RemovePropertyById(&aDecl, aPropertyId, {}); + auto propertyId = CSSPropertyId(aPropertyId); + return Servo_DeclarationBlock_RemovePropertyById(&aDecl, &propertyId, {}); } bool changed = false; diff --git a/layout/style/DeclarationBlock.h b/layout/style/DeclarationBlock.h index a7818d870963..557294ee0c6f 100644 --- a/layout/style/DeclarationBlock.h +++ b/layout/style/DeclarationBlock.h @@ -10,6 +10,7 @@ #ifndef mozilla_DeclarationBlock_h #define mozilla_DeclarationBlock_h +#include "CSSPropertyId.h" #include "NonCustomCSSPropertyId.h" #include "mozilla/ServoBindings.h" #include "nsString.h" @@ -149,7 +150,9 @@ class DeclarationBlock final { bool RemovePropertyById(NonCustomCSSPropertyId aProperty, DeclarationBlockMutationClosure aClosure = {}) { AssertMutable(); - return Servo_DeclarationBlock_RemovePropertyById(mRaw, aProperty, aClosure); + auto propertyId = CSSPropertyId(aProperty); + return Servo_DeclarationBlock_RemovePropertyById(mRaw, &propertyId, + aClosure); } private: diff --git a/layout/style/nsDOMCSSDeclaration.cpp b/layout/style/nsDOMCSSDeclaration.cpp index 17e924b102bf..a543a10b0eda 100644 --- a/layout/style/nsDOMCSSDeclaration.cpp +++ b/layout/style/nsDOMCSSDeclaration.cpp @@ -30,16 +30,22 @@ JSObject* nsDOMCSSDeclaration::WrapObject(JSContext* aCx, NS_IMPL_QUERY_INTERFACE(nsDOMCSSDeclaration, nsICSSDeclaration) +void nsDOMCSSDeclaration::GetPropertyValue(const CSSPropertyId& aPropertyId, + nsACString& aValue) { + MOZ_ASSERT(aPropertyId.IsValid(), "Should pass a valid CSSPropertyId"); + MOZ_ASSERT(aValue.IsEmpty()); + + if (Block* decl = GetOrCreateCSSDeclaration(Operation::Read, nullptr)) { + Servo_DeclarationBlock_GetPropertyValueById(decl, &aPropertyId, &aValue); + } +} + void nsDOMCSSDeclaration::GetPropertyValue(const NonCustomCSSPropertyId aPropId, nsACString& aValue) { MOZ_ASSERT(aPropId != eCSSProperty_UNKNOWN, "Should never pass eCSSProperty_UNKNOWN around"); - MOZ_ASSERT(aValue.IsEmpty()); - if (Block* decl = GetOrCreateCSSDeclaration(Operation::Read, nullptr)) { - Servo_DeclarationBlock_GetPropertyValueByNonCustomId(decl, aPropId, - &aValue); - } + GetPropertyValue(CSSPropertyId(aPropId), aValue); } void nsDOMCSSDeclaration::SetPropertyValue(const NonCustomCSSPropertyId aPropId, @@ -53,7 +59,7 @@ void nsDOMCSSDeclaration::SetPropertyValue(const NonCustomCSSPropertyId aPropId, if (aValue.IsEmpty()) { // If the new value of the property is an empty string we remove the // property. - return RemovePropertyInternal(aPropId, aRv); + return RemoveProperty(CSSPropertyId(aPropId), aRv); } aRv = ParsePropertyValue(aPropId, aValue, false, aSubjectPrincipal); @@ -186,17 +192,17 @@ void nsDOMCSSDeclaration::SetProperty(const nsACString& aPropertyName, return; } + // In the common (and fast) cases we can use the property id + CSSPropertyId propertyId = CSSPropertyId::Parse(aPropertyName); + if (!propertyId.IsValid()) { + return; + } + if (aValue.IsEmpty()) { // If the new value of the property is an empty string we remove the // property. // XXX this ignores the priority string, should it? - return RemovePropertyInternal(aPropertyName, aRv); - } - - // In the common (and fast) cases we can use the property id - NonCustomCSSPropertyId propId = nsCSSProps::LookupProperty(aPropertyName); - if (propId == eCSSProperty_UNKNOWN) { - return; + return RemoveProperty(propertyId, aRv); } bool important; @@ -209,12 +215,13 @@ void nsDOMCSSDeclaration::SetProperty(const nsACString& aPropertyName, return; } - if (propId == eCSSPropertyExtra_variable) { + if (propertyId.IsCustom()) { aRv = ParseCustomPropertyValue(aPropertyName, aValue, important, aSubjectPrincipal); return; } - aRv = ParsePropertyValue(propId, aValue, important, aSubjectPrincipal); + aRv = + ParsePropertyValue(propertyId.mId, aValue, important, aSubjectPrincipal); } void nsDOMCSSDeclaration::RemoveProperty(const nsACString& aPropertyName, @@ -223,8 +230,40 @@ void nsDOMCSSDeclaration::RemoveProperty(const nsACString& aPropertyName, if (IsReadOnly()) { return; } - GetPropertyValue(aPropertyName, aReturn); - RemovePropertyInternal(aPropertyName, aRv); + + CSSPropertyId propertyId = CSSPropertyId::Parse(aPropertyName); + if (!propertyId.IsValid()) { + return; + } + + GetPropertyValue(propertyId, aReturn); + + RemoveProperty(propertyId, aRv); +} + +void nsDOMCSSDeclaration::RemoveProperty(const CSSPropertyId& aPropertyId, + mozilla::ErrorResult& aRv) { + if (IsReadOnly()) { + return; + } + + Block* olddecl = + GetOrCreateCSSDeclaration(Operation::RemoveProperty, nullptr); + if (!olddecl) { + return; // no decl, so nothing to remove + } + + mozAutoDocUpdate autoUpdate(DocToUpdate(), true); + + DeclarationBlockMutationClosure closure = {}; + MutationClosureData closureData; + GetPropertyChangeClosure(&closure, &closureData); + + RefPtr decl = EnsureBlockMutable(olddecl); + if (!Servo_DeclarationBlock_RemovePropertyById(decl, &aPropertyId, closure)) { + return; + } + aRv = SetCSSDeclaration(decl, &closureData); } /* static */ nsDOMCSSDeclaration::ParsingEnvironment @@ -358,36 +397,6 @@ nsresult nsDOMCSSDeclaration::SetPropertyTypedValue( }); } -void nsDOMCSSDeclaration::RemovePropertyInternal(NonCustomCSSPropertyId aPropId, - ErrorResult& aRv) { - Block* olddecl = - GetOrCreateCSSDeclaration(Operation::RemoveProperty, nullptr); - if (IsReadOnly()) { - return; - } - - if (!olddecl) { - return; // no decl, so nothing to remove - } - - // For nsDOMCSSAttributeDeclaration, SetCSSDeclaration will lead to - // Attribute setting code, which leads in turn to BeginUpdate. We - // need to start the update now so that the old rule doesn't get used - // between when we mutate the declaration and when we set the new - // rule (see stack in bug 209575). - mozAutoDocUpdate autoUpdate(DocToUpdate(), true); - - DeclarationBlockMutationClosure closure = {}; - MutationClosureData closureData; - GetPropertyChangeClosure(&closure, &closureData); - - RefPtr decl = EnsureBlockMutable(olddecl); - if (!Servo_DeclarationBlock_RemovePropertyById(decl, aPropId, closure)) { - return; - } - aRv = SetCSSDeclaration(decl, &closureData); -} - already_AddRefed nsDOMCSSDeclaration::EnsureBlockMutable(Block* aBlock) { if (Servo_DeclarationBlock_IsImmutable(aBlock)) { @@ -395,33 +404,3 @@ nsDOMCSSDeclaration::EnsureBlockMutable(Block* aBlock) { } return do_AddRef(aBlock); } - -void nsDOMCSSDeclaration::RemovePropertyInternal( - const nsACString& aPropertyName, ErrorResult& aRv) { - if (IsReadOnly()) { - return; - } - - Block* olddecl = - GetOrCreateCSSDeclaration(Operation::RemoveProperty, nullptr); - if (!olddecl) { - return; // no decl, so nothing to remove - } - - // For nsDOMCSSAttributeDeclaration, SetCSSDeclaration will lead to - // Attribute setting code, which leads in turn to BeginUpdate. We - // need to start the update now so that the old rule doesn't get used - // between when we mutate the declaration and when we set the new - // rule (see stack in bug 209575). - mozAutoDocUpdate autoUpdate(DocToUpdate(), true); - - DeclarationBlockMutationClosure closure = {}; - MutationClosureData closureData; - GetPropertyChangeClosure(&closure, &closureData); - - RefPtr decl = EnsureBlockMutable(olddecl); - if (!Servo_DeclarationBlock_RemoveProperty(decl, &aPropertyName, closure)) { - return; - } - aRv = SetCSSDeclaration(decl, &closureData); -} diff --git a/layout/style/nsDOMCSSDeclaration.h b/layout/style/nsDOMCSSDeclaration.h index 8ccfb1e8722b..fb653303a45e 100644 --- a/layout/style/nsDOMCSSDeclaration.h +++ b/layout/style/nsDOMCSSDeclaration.h @@ -65,6 +65,12 @@ class nsDOMCSSDeclaration : public nsICSSDeclaration { NS_IMETHOD_(MozExternalRefCountType) AddRef() override = 0; NS_IMETHOD_(MozExternalRefCountType) Release() override = 0; + /** + * Typed getter: Get property value by CSSPropertyId. + * This handles both non-custom properties and custom properties (--*). + */ + virtual void GetPropertyValue(const mozilla::CSSPropertyId& aPropertyId, + nsACString& aValue); /** * Method analogous to CSSStyleDeclaration::GetPropertyValue, * which obeys all the same restrictions. @@ -103,6 +109,8 @@ class nsDOMCSSDeclaration : public nsICSSDeclaration { bool HasLonghandProperty(const nsACString& propertyName) override; void RemoveProperty(const nsACString& propertyName, nsACString& _retval, mozilla::ErrorResult& aRv) override; + void RemoveProperty(const mozilla::CSSPropertyId& aPropertyId, + mozilla::ErrorResult& aRv); void GetPropertyPriority(const nsACString& propertyName, nsACString& aPriority) override; void SetProperty(const nsACString& propertyName, const nsACString& value, @@ -186,11 +194,6 @@ class nsDOMCSSDeclaration : public nsICSSDeclaration { nsresult SetPropertyTypedValue(const mozilla::CSSPropertyId& aPropId, const nsACString& aPropValue); - void RemovePropertyInternal(NonCustomCSSPropertyId aPropId, - mozilla::ErrorResult& aRv); - void RemovePropertyInternal(const nsACString& aPropert, - mozilla::ErrorResult& aRv); - virtual void GetPropertyChangeClosure( mozilla::DeclarationBlockMutationClosure* aClosure, mozilla::MutationClosureData* aClosureData) {} diff --git a/layout/style/typedom/StylePropertyMap.cpp b/layout/style/typedom/StylePropertyMap.cpp index 1bd264a7efac..b40604ceb6d0 100644 --- a/layout/style/typedom/StylePropertyMap.cpp +++ b/layout/style/typedom/StylePropertyMap.cpp @@ -40,6 +40,14 @@ struct DeclarationTraits { declaration->SetPropertyTypedValue(aPropertyId, aValue, aRv); } + + static void Delete(nsStyledElement* aStyledElement, + const CSSPropertyId& aPropertyId, ErrorResult& aRv) { + MOZ_ASSERT(aStyledElement); + nsCOMPtr declaration = aStyledElement->Style(); + + declaration->RemoveProperty(aPropertyId, aRv); + } }; // Specialization for style rule @@ -55,6 +63,14 @@ struct DeclarationTraits { declaration->SetPropertyTypedValue(aPropertyId, aValue, aRv); } + + static void Delete(CSSStyleRule* aRule, const CSSPropertyId& aPropertyId, + ErrorResult& aRv) { + MOZ_ASSERT(aRule); + nsCOMPtr declaration = aRule->Style(); + + declaration->RemoveProperty(aPropertyId, aRv); + } }; } // namespace @@ -80,15 +96,13 @@ void StylePropertyMap::Set( const Sequence& aValues, ErrorResult& aRv) { // Step 2. + auto propertyId = CSSPropertyId::Parse(aProperty); - NonCustomCSSPropertyId id = nsCSSProps::LookupProperty(aProperty); - if (id == eCSSProperty_UNKNOWN) { + if (!propertyId.IsValid()) { aRv.ThrowTypeError("Invalid property: "_ns + aProperty); return; } - auto propertyId = CSSPropertyId::FromIdOrCustomProperty(id, aProperty); - if (aValues.Length() != 1) { aRv.Throw(NS_ERROR_NOT_IMPLEMENTED); return; @@ -145,8 +159,27 @@ void StylePropertyMap::Append( aRv.Throw(NS_ERROR_NOT_IMPLEMENTED); } +// https://drafts.css-houdini.org/css-typed-om/#dom-stylepropertymap-delete void StylePropertyMap::Delete(const nsACString& aProperty, ErrorResult& aRv) { - aRv.Throw(NS_ERROR_NOT_IMPLEMENTED); + if (!mParent) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return; + } + + // Step 1. If property is not a custom property name string, set property to + // property ASCII lowercased. + // TODO: Implement Step 1 if it's actually needed. + + // Step 2. If property is not a valid CSS property, throw a TypeError. + auto propertyId = CSSPropertyId::Parse(aProperty); + if (!propertyId.IsValid()) { + aRv.ThrowTypeError("Invalid property: "_ns + aProperty); + return; + } + + // Step 3. If this’s [[declarations]] internal slot contains property, remove + // it. + mDeclarations.Delete(propertyId, aRv); } // https://drafts.css-houdini.org/css-typed-om/#dom-stylepropertymap-clear @@ -203,4 +236,23 @@ void StylePropertyMapReadOnly::Declarations::Clear(ErrorResult& aRv) { } } +void StylePropertyMapReadOnly::Declarations::Delete( + const CSSPropertyId& aPropertyId, ErrorResult& aRv) { + switch (mKind) { + case Kind::Inline: + DeclarationTraits::Delete( + mStyledElement, aPropertyId, aRv); + return; + + case Kind::Computed: + MOZ_ASSERT_UNREACHABLE("ComputedStyleMap is not writable"); + return; + + case Kind::Rule: + DeclarationTraits::Delete(mRule, + aPropertyId, aRv); + return; + } +} + } // namespace mozilla::dom diff --git a/layout/style/typedom/StylePropertyMapReadOnly.h b/layout/style/typedom/StylePropertyMapReadOnly.h index e76aee26164e..5fdbfe4b1a51 100644 --- a/layout/style/typedom/StylePropertyMapReadOnly.h +++ b/layout/style/typedom/StylePropertyMapReadOnly.h @@ -109,6 +109,7 @@ class StylePropertyMapReadOnly : public nsISupports, public nsWrapperCache { void Set(const CSSPropertyId& aPropertyId, const nsACString& aValue, ErrorResult& aRv); void Clear(ErrorResult& aRv); + void Delete(const CSSPropertyId& aPropertyId, ErrorResult& aRv); URLExtraData* GetURLExtraData() const; void Unlink(); diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 721f4f4fca73..309d34b51468 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -5925,14 +5925,12 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_RemoveProperty( #[no_mangle] pub extern "C" fn Servo_DeclarationBlock_RemovePropertyById( declarations: &LockedDeclarationBlock, - property: NonCustomCSSPropertyId, + property_id: &structs::CSSPropertyId, before_change_closure: DeclarationBlockMutationClosure, ) -> bool { - remove_property( - declarations, - get_property_id_from_noncustomcsspropertyid!(property, false), - before_change_closure, - ) + let property_id = get_property_id_from_csspropertyid!(property_id, false); + + remove_property(declarations, property_id, before_change_closure) } #[no_mangle] diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-invalid.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-invalid.html.ini deleted file mode 100644 index bc012601db2e..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-invalid.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[delete-invalid.html] - [Deleting an unsupported property name throws a TypeError] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-shorthand.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-shorthand.html.ini deleted file mode 100644 index f2709f792b43..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete-shorthand.html.ini +++ /dev/null @@ -1,10 +0,0 @@ -[delete-shorthand.html] - [Deleting a shorthand property not in the css rule is a no-op] - expected: FAIL - - [Deleting a shorthand property in the css rule removes both it and its longhands] - expected: FAIL - - [Deleting a longhand property in the css rule removes both it and its shorthand] - expected: FAIL - diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete.html.ini deleted file mode 100644 index 4f5b5b07e0ef..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/delete.html.ini +++ /dev/null @@ -1,16 +0,0 @@ -[delete.html] - [Deleting a property not in the css rule is a no-op] - expected: FAIL - - [Deleting a property in the css rule removes it from the css rule] - expected: FAIL - - [Deleting a custom property in the css rule removes it from the css rule] - expected: FAIL - - [Deleting a list-valued property in the css rule removes it from the css rule] - expected: FAIL - - [Declared StylePropertyMap.delete is not case-sensitive] - expected: FAIL - diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/attribute-changed-callback.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/attribute-changed-callback.html.ini index f2b16fb08a6c..8a190a342bfb 100644 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/attribute-changed-callback.html.ini +++ b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/attribute-changed-callback.html.ini @@ -1,9 +1,4 @@ [attribute-changed-callback.html] - [attributeStyleMap.delete() triggers attributeChangedCallback] - expected: FAIL - [attributeStyleMap.append() triggers attributeChangedCallback] expected: FAIL - [attributeStyleMap.delete() with custom property triggers attributeChangedCallback] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-invalid.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-invalid.html.ini deleted file mode 100644 index bc012601db2e..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-invalid.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[delete-invalid.html] - [Deleting an unsupported property name throws a TypeError] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-shorthand.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-shorthand.html.ini deleted file mode 100644 index 2baa2694979b..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete-shorthand.html.ini +++ /dev/null @@ -1,10 +0,0 @@ -[delete-shorthand.html] - [Deleting a shorthand property not in the inline style is a no-op] - expected: FAIL - - [Deleting a shorthand property in the inline style removes both it and its longhands] - expected: FAIL - - [Deleting a longhand property in the inline style removes both it and its shorthand] - expected: FAIL - diff --git a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete.html.ini b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete.html.ini deleted file mode 100644 index 545feea11654..000000000000 --- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/inline/delete.html.ini +++ /dev/null @@ -1,16 +0,0 @@ -[delete.html] - [Deleting a property not in the inline style is a no-op] - expected: FAIL - - [Deleting a property in the inline style removes it from the inline style] - expected: FAIL - - [Deleting a custom property in the inline style removes it from the inline style] - expected: FAIL - - [Deleting a list-valued property in the inline style removes it from the inline style] - expected: FAIL - - [Inline StylePropertyMap.delete is not case-sensitive] - expected: FAIL -