Title: [273627] trunk
Revision
273627
Author
wei...@apple.com
Date
2021-02-27 11:51:44 -0800 (Sat, 27 Feb 2021)

Log 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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (273626 => 273627)


--- trunk/LayoutTests/ChangeLog	2021-02-27 15:34:20 UTC (rev 273626)
+++ trunk/LayoutTests/ChangeLog	2021-02-27 19:51:44 UTC (rev 273627)
@@ -1,3 +1,14 @@
+2021-02-27  Sam Weinig  <wei...@apple.com>
+
+       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.
+
 2021-02-27  Rob Buis  <rb...@igalia.com>
 
         Null check ArrayBufferView RefPtr

Modified: trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt (273626 => 273627)


--- trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt	2021-02-27 15:34:20 UTC (rev 273626)
+++ trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt	2021-02-27 19:51:44 UTC (rev 273627)
@@ -3,24 +3,8 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-normal cases
-
 PASS element.style.zIndex is '1'
 PASS element.style.ZIndex is undefined.
-
-"pixel" prefix
-
-PASS element.style.pixelZIndex is 1
-PASS element.style.PixelZIndex is 1
-PASS element.style.pixelzIndex is undefined.
-PASS element.style.PixelzIndex is undefined.
-
-"pos" prefix
-
-PASS element.style.posZIndex is 1
-PASS element.style.PosZIndex is 1
-PASS element.style.poszIndex is undefined.
-PASS element.style.PoszIndex is undefined.
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html (273626 => 273627)


--- trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html	2021-02-27 15:34:20 UTC (rev 273626)
+++ trunk/LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html	2021-02-27 19:51:44 UTC (rev 273627)
@@ -12,29 +12,8 @@
 var element = document.createElement('a');
 element.style.zIndex = 1;
 
-debug('normal cases');
-debug('');
-
 shouldBe("element.style.zIndex", "'1'");
 shouldBeUndefined("element.style.ZIndex");
-
-debug('');
-debug('"pixel" prefix');
-debug('');
-
-shouldBe("element.style.pixelZIndex", "1");
-shouldBe("element.style.PixelZIndex", "1");
-shouldBeUndefined("element.style.pixelzIndex");
-shouldBeUndefined("element.style.PixelzIndex");
-
-debug('');
-debug('"pos" prefix');
-debug('');
-
-shouldBe("element.style.posZIndex", "1");
-shouldBe("element.style.PosZIndex", "1");
-shouldBeUndefined("element.style.poszIndex");
-shouldBeUndefined("element.style.PoszIndex");
 </script>
 <script src=""
 </body>

Modified: trunk/Source/WebCore/ChangeLog (273626 => 273627)


--- trunk/Source/WebCore/ChangeLog	2021-02-27 15:34:20 UTC (rev 273626)
+++ trunk/Source/WebCore/ChangeLog	2021-02-27 19:51:44 UTC (rev 273627)
@@ -1,3 +1,19 @@
+2021-02-27  Sam Weinig  <wei...@apple.com>
+
+        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):
+
 2021-02-27  Zalan Bujtas  <za...@apple.com>
 
         [LFC][Coverage] Add missing not-yet-modern-line-layout reasons

Modified: trunk/Source/WebCore/css/CSSStyleDeclaration.cpp (273626 => 273627)


--- trunk/Source/WebCore/css/CSSStyleDeclaration.cpp	2021-02-27 15:34:20 UTC (rev 273626)
+++ trunk/Source/WebCore/css/CSSStyleDeclaration.cpp	2021-02-27 19:51:44 UTC (rev 273627)
@@ -43,13 +43,7 @@
 
 namespace {
 
-enum class PropertyNamePrefix {
-    None,
-    Epub,
-    Pixel,
-    Pos,
-    WebKit
-};
+enum class PropertyNamePrefix { None, Epub, WebKit };
 
 template<size_t prefixCStringLength>
 static inline bool matchesCSSPropertyNamePrefix(const StringImpl& propertyName, const char (&prefix)[prefixCStringLength])
@@ -93,12 +87,6 @@
         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':
         if (matchesCSSPropertyNamePrefix(propertyName, "webkit"))
             return PropertyNamePrefix::WebKit;
@@ -131,31 +119,22 @@
     *buffer++ = '-';
 }
 
-struct CSSPropertyInfo {
-    CSSPropertyID propertyID;
-    bool hadPixelOrPosPrefix;
-};
-
-static CSSPropertyInfo parseJavaScriptCSSPropertyName(const AtomString& propertyName)
+static CSSPropertyID parseJavaScriptCSSPropertyName(const AtomString& propertyName)
 {
-    using CSSPropertyInfoMap = HashMap<String, CSSPropertyInfo>;
-    static NeverDestroyed<CSSPropertyInfoMap> propertyInfoCache;
+    using CSSPropertyIDMap = HashMap<String, CSSPropertyID>;
+    static NeverDestroyed<CSSPropertyIDMap> propertyIDCache;
 
-    CSSPropertyInfo propertyInfo = { CSSPropertyInvalid, false };
-
     auto* propertyNameString = propertyName.impl();
     if (!propertyNameString)
-        return propertyInfo;
+        return CSSPropertyInvalid;
+
     unsigned length = propertyNameString->length();
     if (!length)
-        return propertyInfo;
+        return CSSPropertyInvalid;
 
-    propertyInfo = propertyInfoCache.get().get(propertyNameString);
-    if (propertyInfo.propertyID)
-        return propertyInfo;
+    if (auto id = propertyIDCache.get().get(propertyNameString))
+        return id;
 
-    bool hadPixelOrPosPrefix = false;
-
     constexpr size_t bufferSize = maxCSSPropertyNameLength + 1;
     char buffer[bufferSize];
     char* bufferPtr = buffer;
@@ -162,22 +141,11 @@
     const char* name = bufferPtr;
 
     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;
+            return CSSPropertyInvalid;
         break;
-    case PropertyNamePrefix::Pixel:
-        i += 5;
-        hadPixelOrPosPrefix = true;
-        break;
-    case PropertyNamePrefix::Pos:
-        i += 3;
-        hadPixelOrPosPrefix = true;
-        break;
     case PropertyNamePrefix::Epub:
         writeEpubPrefix(bufferPtr);
         i += 4;
@@ -195,17 +163,17 @@
     size_t bufferSizeLeft = stringEnd - bufferPtr;
     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);
         } else
@@ -220,23 +188,24 @@
     cssPropertyNameIOSAliasing(buffer, name, outputLength);
 #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;
+    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 CSSPropertyInfo propertyInfoFromJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settings)
+static CSSPropertyID propertyIDFromJavaScriptCSSPropertyName(const AtomString& propertyName, const Settings* settings)
 {
-    auto propertyInfo = parseJavaScriptCSSPropertyName(propertyName);
-    auto id = propertyInfo.propertyID;
+    auto id = parseJavaScriptCSSPropertyName(propertyName);
     if (!isEnabledCSSProperty(id) || !isCSSPropertyEnabledBySettings(id, settings))
-        return { CSSPropertyInvalid, false };
-    return propertyInfo;
+        return CSSPropertyInvalid;
+    return id;
 }
 
 }
@@ -243,7 +212,8 @@
 
 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);
 }
 
 const Settings* CSSStyleDeclaration::settings() const
@@ -253,30 +223,23 @@
 
 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) };
-    }
+    if (auto value = getPropertyCSSValueInternal(propertyID))
+        return Variant<String, double> { value->cssText() };
 
-    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,9 +246,6 @@
 
     propertySupported = true;
 
-    if (propertyInfo.hadPixelOrPosPrefix)
-        value.append("px");
-
     bool important = false;
     if (DeprecatedGlobalSettings::shouldRespectPriorityInCSSAttributeSetters()) {
         auto importantIndex = value.findIgnoringASCIICase("!important");
@@ -295,7 +255,7 @@
         }
     }
 
-    auto setPropertyInternalResult = setPropertyInternal(propertyInfo.propertyID, value, important);
+    auto setPropertyInternalResult = setPropertyInternal(propertyID, value, important);
     if (setPropertyInternalResult.hasException())
         return setPropertyInternalResult.releaseException();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to