Attachment 421729 Details for Bug 119712 – Patch

[patch] Patch

bug-119712-20210226165729.patch (text/plain), 12.60 KB, created by Sam Weinig on 2021-02-26 16:57:30 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sam Weinig

Created: 2021-02-26 16:57:30 PST

Size: 12.60 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 273605)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2021-02-26  Sam Weinig  <weinig@apple.com>
>+
>+        Remove support for 'pixel' and 'pos' CSSOM prefixes
>+        https://bugs.webkit.org/show_bug.cgi?id=119712
>+        <rdar://problem/70660490>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-26  Jean-Yves Avenard  <jya@apple.com>
> 
>         [MSE] WebContent/GPU process will hang when appending data under some circumstances.
>Index: Source/WebCore/css/CSSStyleDeclaration.cpp
>===================================================================
>--- Source/WebCore/css/CSSStyleDeclaration.cpp	(revision 273603)
>+++ Source/WebCore/css/CSSStyleDeclaration.cpp	(working copy)
>@@ -43,13 +43,7 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(CSSStyleDecl
> 
> 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 @@ static PropertyNamePrefix propertyNamePr
>         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,30 +119,23 @@ static inline void writeEpubPrefix(char*
>     *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 };
>+    CSSPropertyID propertyID = CSSPropertyInvalid;
> 
>     auto* propertyNameString = propertyName.impl();
>     if (!propertyNameString)
>-        return propertyInfo;
>+        return propertyID;
>     unsigned length = propertyNameString->length();
>     if (!length)
>-        return propertyInfo;
>-
>-    propertyInfo = propertyInfoCache.get().get(propertyNameString);
>-    if (propertyInfo.propertyID)
>-        return propertyInfo;
>+        return propertyID;
> 
>-    bool hadPixelOrPosPrefix = false;
>+    propertyID = propertyIDCache.get().get(propertyNameString);
>+    if (propertyID)
>+        return propertyID;
> 
>     constexpr size_t bufferSize = maxCSSPropertyNameLength + 1;
>     char buffer[bufferSize];
>@@ -162,21 +143,12 @@ static CSSPropertyInfo parseJavaScriptCS
>     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;
>-        break;
>-    case PropertyNamePrefix::Pixel:
>-        i += 5;
>-        hadPixelOrPosPrefix = true;
>-        break;
>-    case PropertyNamePrefix::Pos:
>-        i += 3;
>-        hadPixelOrPosPrefix = true;
>+            return propertyID;
>         break;
>     case PropertyNamePrefix::Epub:
>         writeEpubPrefix(bufferPtr);
>@@ -195,17 +167,17 @@ static CSSPropertyInfo parseJavaScriptCS
>     size_t bufferSizeLeft = stringEnd - bufferPtr;
>     size_t propertySizeLeft = length - i;
>     if (propertySizeLeft > bufferSizeLeft)
>-        return propertyInfo;
>+        return propertyID;
> 
>     for (; i < length; ++i) {
>         UChar c = (*propertyNameString)[i];
>         if (!c || !isASCII(c))
>-            return propertyInfo; // illegal character
>+            return propertyID; // illegal character
>         if (isASCIIUpper(c)) {
>             size_t bufferSizeLeft = stringEnd - bufferPtr;
>             size_t propertySizeLeft = length - i + 1;
>             if (propertySizeLeft > bufferSizeLeft)
>-                return propertyInfo;
>+                return propertyID;
>             *bufferPtr++ = '-';
>             *bufferPtr++ = toASCIILowerUnchecked(c);
>         } else
>@@ -221,29 +193,25 @@ static CSSPropertyInfo parseJavaScriptCS
> #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;
>+    if (auto propertyID = hashTableEntry ? hashTableEntry->id : 0)
>+        propertyIDCache.get().add(propertyNameString, static_cast<CSSPropertyID>(propertyID));
>+    return propertyID;
> }
> 
>-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;
> }
> 
> }
> 
> 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,39 +221,29 @@ const Settings* CSSStyleDeclaration::set
> 
> 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 { };
>     }
> 
>     propertySupported = true;
> 
>-    if (propertyInfo.hadPixelOrPosPrefix)
>-        value.append("px");
>-
>     bool important = false;
>     if (DeprecatedGlobalSettings::shouldRespectPriorityInCSSAttributeSetters()) {
>         auto importantIndex = value.findIgnoringASCIICase("!important");
>@@ -295,7 +253,7 @@ ExceptionOr<void> CSSStyleDeclaration::s
>         }
>     }
> 
>-    auto setPropertyInternalResult = setPropertyInternal(propertyInfo.propertyID, value, important);
>+    auto setPropertyInternalResult = setPropertyInternal(propertyID, value, important);
>     if (setPropertyInternalResult.hasException())
>         return setPropertyInternalResult.releaseException();
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 273603)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2021-02-26  Sam Weinig  <weinig@apple.com>
>+
>+       Remove support for 'pixel' and 'pos' CSSOM prefixes
>+       https://bugs.webkit.org/show_bug.cgi?id=119712
>+
>+       Reviewed by NOBODY (OOPS!).
>+
>+       * 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-26  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Coverage] Do not bail out on text underline position when text-decoration is 'none'
>Index: LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt
>===================================================================
>--- LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt	(revision 273603)
>+++ LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive-expected.txt	(working copy)
>@@ -3,25 +3,8 @@ This test checks that access to CSS prop
> 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
> 
>Index: LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html
>===================================================================
>--- LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html	(revision 273603)
>+++ LayoutTests/fast/dom/CSSStyleDeclaration/css-properties-case-sensitive.html	(working copy)
>@@ -12,29 +12,8 @@ description(
> 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="../../../resources/js-test-post.js"></script>
> </body>

Flags:

ews-feeder: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 119712: 208584 | 412209 | 412210 | 412241 | 421728 | 421729 | 421732 | 421733 | 421748