tor-browser

commit f6a33ca75bf5a4774e615855ed9c12b6696be268
parent ae90c334421ac2b61272b86d80f60ad91779ad60
Author: Jan Varga <jan.varga@gmail.com>
Date:   Sun, 16 Nov 2025 12:15:26 +0000

Bug 1998939 - Add safe CSSPropertyId factory helpers and fix a mis construction; r=emilio,firefox-style-system-reviewers

Introduce new factory helper methods on CSSPropertyId to safely construct a
property identifier from either a non custom property id or a custom property.

The patch also fixes an existing mis construction in
nsDOMWindowUtils::ComputeAnimationDistance, which incorrectly built a
CSSPropertyId for custom properties.

Differential Revision: https://phabricator.services.mozilla.com/D271787

Diffstat:
M
dom/animation/KeyframeUtils.cpp
 | 
7
+------
M
dom/base/nsDOMWindowUtils.cpp
 | 
20
+++++++++-----------
M
layout/style/CSSPropertyId.h
 | 
32
++++++++++++++++++++++++++++++++
M
layout/style/nsTransitionManager.cpp
 | 
3
++-

4 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/dom/animation/KeyframeUtils.cpp b/dom/animation/KeyframeUtils.cpp
@@ -569,12 +569,7 @@ static bool GetPropertyValuesPairs(JSContext* aCx,
           propName, CSSEnabledState::ForAllContent);
     }
 
-    // TODO(zrhoffman, bug 1811897) Add test coverage for removing the `--`
-    // prefix here.
-    CSSPropertyId property = propertyId == eCSSPropertyExtra_variable
-                                 ? CSSPropertyId(NS_Atomize(Substring(
-                                       propName, 2, propName.Length() - 2)))
-                                 : CSSPropertyId(propertyId);
+    auto property = CSSPropertyId::FromIdOrCustomProperty(propertyId, propName);
 
     if (KeyframeUtils::IsAnimatableProperty(property)) {
       properties.AppendElement(AdditionalProperty{std::move(property), i});
diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp
@@ -3256,16 +3256,15 @@ nsDOMWindowUtils::ComputeAnimationDistance(Element* aElement,
                                            double* aResult) {
   NS_ENSURE_ARG_POINTER(aElement);
 
-  NonCustomCSSPropertyId propertyId =
-      nsCSSProps::LookupProperty(NS_ConvertUTF16toUTF8(aProperty));
+  NS_ConvertUTF16toUTF8 prop(aProperty);
+
+  NonCustomCSSPropertyId propertyId = nsCSSProps::LookupProperty(prop);
   if (propertyId == eCSSProperty_UNKNOWN ||
       nsCSSProps::IsShorthand(propertyId)) {
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
-  CSSPropertyId property = propertyId == eCSSPropertyExtra_variable
-                               ? CSSPropertyId(NS_Atomize(aProperty))
-                               : CSSPropertyId(propertyId);
+  auto property = CSSPropertyId::FromIdOrCustomProperty(propertyId, prop);
 
   AnimationValue v1 = AnimationValue::FromString(
       property, NS_ConvertUTF16toUTF8(aValue1), aElement);
@@ -3289,16 +3288,15 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement,
     return NS_ERROR_INVALID_ARG;
   }
 
-  NonCustomCSSPropertyId propertyId =
-      nsCSSProps::LookupProperty(NS_ConvertUTF16toUTF8(aProperty));
+  NS_ConvertUTF16toUTF8 prop(aProperty);
+
+  NonCustomCSSPropertyId propertyId = nsCSSProps::LookupProperty(prop);
   if (propertyId == eCSSProperty_UNKNOWN ||
       nsCSSProps::IsShorthand(propertyId)) {
     return NS_ERROR_INVALID_ARG;
   }
-  CSSPropertyId property = propertyId == eCSSPropertyExtra_variable
-                               ? CSSPropertyId(NS_Atomize(Substring(
-                                     aProperty, 2, aProperty.Length() - 2)))
-                               : CSSPropertyId(propertyId);
+
+  auto property = CSSPropertyId::FromIdOrCustomProperty(propertyId, prop);
 
   switch (aFlushType) {
     case FLUSH_NONE:
diff --git a/layout/style/CSSPropertyId.h b/layout/style/CSSPropertyId.h
@@ -8,8 +8,11 @@
 #define mozilla_CSSPropertyId_h
 
 #include "NonCustomCSSPropertyId.h"
+#include "mozilla/Assertions.h"
 #include "mozilla/HashFunctions.h"
+#include "mozilla/RefPtr.h"
 #include "mozilla/ServoBindings.h"
+#include "nsAtom.h"
 #include "nsCSSProps.h"
 #include "nsString.h"
 
@@ -22,11 +25,40 @@ struct CSSPropertyId {
                "eCSSPropertyExtra_variable.");
   }
 
+ private:
   explicit CSSPropertyId(RefPtr<nsAtom> aCustomName)
       : mId(eCSSPropertyExtra_variable), mCustomName(std::move(aCustomName)) {
     MOZ_ASSERT(mCustomName, "Null custom property name");
   }
 
+ public:
+  // Callers are expected to pass a custom name atom with the leading "--"
+  // already stripped. The remaining ident may legally start with "-"
+  // (or even "--"), so we cannot assert anything about its prefix here.
+  static CSSPropertyId FromCustomName(RefPtr<nsAtom> aCustomName) {
+    return CSSPropertyId(aCustomName);
+  }
+
+  static CSSPropertyId FromCustomProperty(const nsACString& aCustomProperty) {
+    MOZ_ASSERT(StringBeginsWith(aCustomProperty, "--"_ns));
+
+    // TODO(zrhoffman, bug 1811897) Add test coverage for removing the `--`
+    // prefix here.
+    RefPtr<nsAtom> atom =
+        NS_Atomize(Substring(aCustomProperty, 2, aCustomProperty.Length() - 2));
+
+    return FromCustomName(atom);
+  }
+
+  static CSSPropertyId FromIdOrCustomProperty(
+      NonCustomCSSPropertyId aId, const nsACString& aCustomProperty) {
+    if (aId != eCSSPropertyExtra_variable) {
+      return CSSPropertyId(aId);
+    }
+
+    return FromCustomProperty(aCustomProperty);
+  }
+
   NonCustomCSSPropertyId mId = eCSSProperty_UNKNOWN;
   RefPtr<nsAtom> mCustomName;
 
diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp
@@ -65,7 +65,8 @@ static void ExpandTransitionProperty(const StyleTransitionProperty& aProperty,
     case StyleTransitionProperty::Tag::Unsupported:
       break;
     case StyleTransitionProperty::Tag::Custom: {
-      CSSPropertyId property(aProperty.AsCustom().AsAtom());
+      auto property =
+          CSSPropertyId::FromCustomName(aProperty.AsCustom().AsAtom());
       aHandler(property);
       break;
     }