Bug 2057406 - Implement StylePropertyMap.delete in CSS Typed OM r=janv,firefox-style-system-reviewers,emilio

Differential Revision: https://phabricator.services.mozilla.com/D313936
This commit is contained in:
tannal
2026-07-27 10:59:43 +00:00
committed by ealvarez@mozilla.com
parent aee531ac41
commit cadda9dc89
14 changed files with 132 additions and 158 deletions
+2 -1
View File
@@ -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;
+4 -1
View File
@@ -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:
+56 -77
View File
@@ -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<Block> 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<Block> decl = EnsureBlockMutable(olddecl);
if (!Servo_DeclarationBlock_RemovePropertyById(decl, aPropId, closure)) {
return;
}
aRv = SetCSSDeclaration(decl, &closureData);
}
already_AddRefed<StyleLockedDeclarationBlock>
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<Block> decl = EnsureBlockMutable(olddecl);
if (!Servo_DeclarationBlock_RemoveProperty(decl, &aPropertyName, closure)) {
return;
}
aRv = SetCSSDeclaration(decl, &closureData);
}
+8 -5
View File
@@ -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) {}
+57 -5
View File
@@ -40,6 +40,14 @@ struct DeclarationTraits<MutableInlineStyleDeclarations> {
declaration->SetPropertyTypedValue(aPropertyId, aValue, aRv);
}
static void Delete(nsStyledElement* aStyledElement,
const CSSPropertyId& aPropertyId, ErrorResult& aRv) {
MOZ_ASSERT(aStyledElement);
nsCOMPtr<nsDOMCSSDeclaration> declaration = aStyledElement->Style();
declaration->RemoveProperty(aPropertyId, aRv);
}
};
// Specialization for style rule
@@ -55,6 +63,14 @@ struct DeclarationTraits<MutableStyleRuleDeclarations> {
declaration->SetPropertyTypedValue(aPropertyId, aValue, aRv);
}
static void Delete(CSSStyleRule* aRule, const CSSPropertyId& aPropertyId,
ErrorResult& aRv) {
MOZ_ASSERT(aRule);
nsCOMPtr<nsDOMCSSDeclaration> declaration = aRule->Style();
declaration->RemoveProperty(aPropertyId, aRv);
}
};
} // namespace
@@ -80,15 +96,13 @@ void StylePropertyMap::Set(
const Sequence<OwningCSSStyleValueOrUTF8String>& 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 thiss [[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<MutableInlineStyleDeclarations>::Delete(
mStyledElement, aPropertyId, aRv);
return;
case Kind::Computed:
MOZ_ASSERT_UNREACHABLE("ComputedStyleMap is not writable");
return;
case Kind::Rule:
DeclarationTraits<MutableStyleRuleDeclarations>::Delete(mRule,
aPropertyId, aRv);
return;
}
}
} // namespace mozilla::dom
@@ -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();
+4 -6
View File
@@ -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]
@@ -1,3 +0,0 @@
[delete-invalid.html]
[Deleting an unsupported property name throws a TypeError]
expected: FAIL
@@ -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
@@ -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
@@ -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
@@ -1,3 +0,0 @@
[delete-invalid.html]
[Deleting an unsupported property name throws a TypeError]
expected: FAIL
@@ -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
@@ -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