Changeset 273627 in webkit for trunk/Source/WebCore/css/CSSStyleDeclaration.cpp

Timestamp:

Feb 27, 2021, 11:51:44 AM (4 years ago)

Author:

[email protected]

Message:

Source/WebCore:
Remove support for 'pixel' and 'pos' CSSOM prefixes
​https://bugs.webkit.org/show_bug.cgi?id=119712
<rdar://problem/70660490>

Reviewed by Simon Fraser.

Remove support for pixel/pos prefixed properties of CSSStyleDeclaration which
are no longer supported by any other browser.

• css/CSSStyleDeclaration.cpp:

(WebCore::CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName):
(WebCore::CSSStyleDeclaration::namedItem):
(WebCore::CSSStyleDeclaration::setNamedItem):

LayoutTests:

Remove support for 'pixel' and 'pos' CSSOM prefixes
​https://bugs.webkit.org/show_bug.cgi?id=119712

Reviewed by Simon Fraser.

• fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt:
• fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html:

Update test to reflect removal of pos/pixel prefixes.

File:

• trunk/Source/WebCore/css/CSSStyleDeclaration.cpp (modified) (10 diffs)

trunk/Source/WebCore/css/CSSStyleDeclaration.cpp

@@ -44 +44 @@
 namespace {
 
-enum class PropertyNamePrefix {
-    None,
-    Epub,
-    Pixel,
-    Pos,
-    WebKit
-};
+enum class PropertyNamePrefix { None, Epub, WebKit };
 
 template<size_t prefixCStringLength>
@@ -93 +87 @@
         if (matchesCSSPropertyNamePrefix(propertyName, "epub"))
             return PropertyNamePrefix::Epub;
-        break;
-    case 'p':
-        if (matchesCSSPropertyNamePrefix(propertyName, "pos"))
-            return PropertyNamePrefix::Pos;
-        if (matchesCSSPropertyNamePrefix(propertyName, "pixel"))
-            return PropertyNamePrefix::Pixel;
         break;
     case 'w':
@@ -132 +120 @@
 }
 
-struct CSSPropertyInfo {
-    CSSPropertyID propertyID;
-    bool hadPixelOrPosPrefix;
-};
-
-static CSSPropertyInfo parseJavaScriptCSSPropertyName(const AtomString& propertyName)
-{
-    using CSSPropertyInfoMap = HashMap<String, CSSPropertyInfo>;
-    static NeverDestroyed<CSSPropertyInfoMap> propertyInfoCache;
-
-    CSSPropertyInfo propertyInfo = { CSSPropertyInvalid, false };
+static CSSPropertyID parseJavaScriptCSSPropertyName(const AtomString& propertyName)
+{
+    using CSSPropertyIDMap = HashMap<String, CSSPropertyID>;
+    static NeverDestroyed<CSSPropertyIDMap> propertyIDCache;
 
     auto* propertyNameString = propertyName.impl();
     if (!propertyNameString)
-        return propertyInfo;
+        return CSSPropertyInvalid;
+
     unsigned length = propertyNameString->length();
     if (!length)
-        return propertyInfo;
-
-    propertyInfo = propertyInfoCache.get().get(propertyNameString);
-    if (propertyInfo.propertyID)
-        return propertyInfo;
-
-    bool hadPixelOrPosPrefix = false;
+        return CSSPropertyInvalid;
+
+    if (auto id = propertyIDCache.get().get(propertyNameString))
+        return id;
 
     constexpr size_t bufferSize = maxCSSPropertyNameLength + 1;
@@ -163 +142 @@
 
     unsigned i = 0;
-    // Prefixes Pixel and Pos are ignored.
-    // Prefixes Apple, KHTML and Webkit are transposed to "-webkit-".
-    // The prefix "Epub" becomes "-epub-".
     switch (propertyNamePrefix(*propertyNameString)) {
     case PropertyNamePrefix::None:
         if (isASCIIUpper((*propertyNameString)[0]))
-            return propertyInfo;
-        break;
-    case PropertyNamePrefix::Pixel:
-        i += 5;
-        hadPixelOrPosPrefix = true;
-        break;
-    case PropertyNamePrefix::Pos:
-        i += 3;
-        hadPixelOrPosPrefix = true;
+            return CSSPropertyInvalid;
         break;
     case PropertyNamePrefix::Epub:
@@ -196 +164 @@
     size_t propertySizeLeft = length - i;
     if (propertySizeLeft > bufferSizeLeft)
-        return propertyInfo;
+        return CSSPropertyInvalid;
 
     for (; i < length; ++i) {
         UChar c = (*propertyNameString)[i];
         if (!c || !isASCII(c))
-            return propertyInfo; // illegal character
+            return CSSPropertyInvalid; // illegal character
         if (isASCIIUpper(c)) {
             size_t bufferSizeLeft = stringEnd - bufferPtr;
             size_t propertySizeLeft = length - i + 1;
             if (propertySizeLeft > bufferSizeLeft)
-                return propertyInfo;
+                return CSSPropertyInvalid;
             *bufferPtr++ = '-';
             *bufferPtr++ = toASCIILowerUnchecked(c);
@@ -221 +189 @@
 #endif
 
-    auto* hashTableEntry = findProperty(name, outputLength);
-    if (auto propertyID = hashTableEntry ? hashTableEntry->id : 0) {
-        auto id = static_cast<CSSPropertyID>(propertyID);
-        propertyInfo.hadPixelOrPosPrefix = hadPixelOrPosPrefix;
-        propertyInfo.propertyID = id;
-        propertyInfoCache.get().add(propertyNameString, propertyInfo);
-    }
-    return propertyInfo;
-}
-
-static CSSPropertyInfo propertyInfoFromJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settings)
-{
-    auto propertyInfo = parseJavaScriptCSSPropertyName(propertyName);
-    auto id = propertyInfo.propertyID;
+    auto hashTableEntry = findProperty(name, outputLength);
+    if (!hashTableEntry)
+        return CSSPropertyInvalid;
+
+    auto id = static_cast<CSSPropertyID>(hashTableEntry->id);
+    if (!id)
+        return CSSPropertyInvalid;
+
+    propertyIDCache.get().add(propertyNameString, id);
+    return id;
+}
+
+static CSSPropertyID propertyIDFromJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settings)
+{
+    auto id = parseJavaScriptCSSPropertyName(propertyName);
     if (!isEnabledCSSProperty(id) || !isCSSPropertyEnabledBySettings(id, settings))
-        return { CSSPropertyInvalid, false };
-    return propertyInfo;
+        return CSSPropertyInvalid;
+    return id;
 }
 
@@ -244 +213 @@
 CSSPropertyID CSSStyleDeclaration::getCSSPropertyIDFromJavaScriptPropertyName(const AtomString& propertyName)
 {
-    return propertyInfoFromJavaScriptCSSPropertyName(propertyName, nullptr).propertyID;
+    // FIXME: This is going to return incorrect results for css properties disabled by Settings.
+    return propertyIDFromJavaScriptCSSPropertyName(propertyName, nullptr);
 }
 
@@ -254 +224 @@
 Optional<Variant<String, double>> CSSStyleDeclaration::namedItem(const AtomString& propertyName)
 {
-    auto propertyInfo = propertyInfoFromJavaScriptCSSPropertyName(propertyName, settings());
-    if (!propertyInfo.propertyID)
+    // FIXME: This is going to return incorrect results for css properties disabled by Settings if settings() returns nullptr.
+    auto propertyID = propertyIDFromJavaScriptCSSPropertyName(propertyName, settings());
+    if (!propertyID)
         return WTF::nullopt;
 
-    auto value = getPropertyCSSValueInternal(propertyInfo.propertyID);
-    if (!value) {
-        // If the property is a shorthand property (such as "padding"), it can only be accessed using getPropertyValue.
-        return Variant<String, double> { getPropertyValueInternal(propertyInfo.propertyID) };
-    }
-    
-    if (propertyInfo.hadPixelOrPosPrefix && is<CSSPrimitiveValue>(*value)) {
-        // Call this version of the getter so that, e.g., pixelTop returns top as a number
-        // in pixel units and posTop should does the same _if_ this is a positioned element.
-        // FIXME: If not a positioned element, MSIE documentation says posTop should return 0; this rule is not implemented.
-        return Variant<String, double> { downcast<CSSPrimitiveValue>(*value).floatValue(CSSUnitType::CSS_PX) };
-    }
-
-    return Variant<String, double> { value->cssText() };
+    if (auto value = getPropertyCSSValueInternal(propertyID))
+        return Variant<String, double> { value->cssText() };
+
+    // If the property is a shorthand property (such as "padding"), it can only be accessed using getPropertyValue.
+    return Variant<String, double> { getPropertyValueInternal(propertyID) };
 }
 
 ExceptionOr<void> CSSStyleDeclaration::setNamedItem(const AtomString& propertyName, String value, bool& propertySupported)
 {
-    auto propertyInfo = propertyInfoFromJavaScriptCSSPropertyName(propertyName, settings());
-    if (!propertyInfo.propertyID) {
+    // FIXME: This is going to return incorrect results for css properties disabled by Settings if settings() returns nullptr.
+    auto propertyID = propertyIDFromJavaScriptCSSPropertyName(propertyName, settings());
+    if (!propertyID) {
         propertySupported = false;
         return { };
@@ -283 +246 @@
 
     propertySupported = true;
-
-    if (propertyInfo.hadPixelOrPosPrefix)
-        value.append("px");
 
     bool important = false;
@@ -296 +256 @@
     }
 
-    auto setPropertyInternalResult = setPropertyInternal(propertyInfo.propertyID, value, important);
+    auto setPropertyInternalResult = setPropertyInternal(propertyID, value, important);
     if (setPropertyInternalResult.hasException())
         return setPropertyInternalResult.releaseException();