Attachment 340575 Details for Bug 177684 – Patch

[patch] Patch

bug-177684-20180517141041.patch (text/plain), 6.47 KB, created by Rob Buis on 2018-05-17 05:10:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Buis

Created: 2018-05-17 05:10:42 PDT

Size: 6.47 KB

patch

obsolete

>Subversion Revision: 231889
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c59b643ce2c19e91cb64cb9e4fc03dbc073d392a..88df16cb1ea498c874de2fb5a562c365ab6d7fab 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2018-05-17  Rob Buis  <rbuis@igalia.com>
>+
>+        Cannot unset transition with important
>+        https://bugs.webkit.org/show_bug.cgi?id=177684
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make removeProperty remove the property regardless
>+        if it was set using !important [1].
>+
>+        The new behavior is consistent with Firefox, Chrome and Edge.
>+
>+        [1] https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-removeproperty
>+
>+        Test: fast/css/cssom-remove-important-shorthand-property.html
>+
>+        * css/StyleProperties.cpp:
>+        (WebCore::MutableStyleProperties::removePropertiesInSet):
>+
> 2018-05-16  Devin Rousso  <webkit@devinrousso.com>
> 
>         Web Inspector: create a navigation item for toggling the overlay rulers/guides
>diff --git a/Source/WebCore/css/StyleProperties.cpp b/Source/WebCore/css/StyleProperties.cpp
>index 3c8ce43b357d2c4ed787f79c5b0684df7864d260..30dbeb42b6b5a67a06fa9c65e0fdfc6deccf9086 100644
>--- a/Source/WebCore/css/StyleProperties.cpp
>+++ b/Source/WebCore/css/StyleProperties.cpp
>@@ -1227,8 +1227,7 @@ bool MutableStyleProperties::removePropertiesInSet(const CSSPropertyID* set, uns
>         toRemove.add(set[i]);
> 
>     return m_propertyVector.removeAllMatching([&toRemove] (const CSSProperty& property) {
>-        // Not quite sure if the isImportant test is needed but it matches the existing behavior.
>-        return !property.isImportant() && toRemove.contains(property.id());
>+        return toRemove.contains(property.id());
>     }) > 0;
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 14c0afc06fd46c38830fe62d643ad5adb5b2ccdc..7c484c93bb5ef05d686be22dab1e7efe4f5e4229 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-05-17  Rob Buis  <rbuis@igalia.com>
>+
>+        Cannot unset transition with important
>+        https://bugs.webkit.org/show_bug.cgi?id=177684
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/css/cssom-remove-important-shorthand-property-expected.txt: Added.
>+        * fast/css/cssom-remove-important-shorthand-property.html: Added.
>+
> 2018-05-16  Brady Eidson  <beidson@apple.com>
> 
>         LayoutTest storage/indexeddb/dont-wedge-private.html is a flaky failure
>diff --git a/LayoutTests/fast/css/cssom-remove-important-shorthand-property-expected.txt b/LayoutTests/fast/css/cssom-remove-important-shorthand-property-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..46f231589a02cbfce8681dfa9de614f240ff23fc
>--- /dev/null
>+++ b/LayoutTests/fast/css/cssom-remove-important-shorthand-property-expected.txt
>@@ -0,0 +1,49 @@
>+Tests that shorthand properties can be removed via CSSOM.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS canSetProperty('font') is true
>+PASS canRemoveProperty('font') is true
>+PASS canSetProperty('border-top') is true
>+PASS canRemoveProperty('border-top') is true
>+PASS canSetProperty('border-right') is true
>+PASS canRemoveProperty('border-right') is true
>+PASS canSetProperty('border-bottom') is true
>+PASS canRemoveProperty('border-bottom') is true
>+PASS canSetProperty('border-left') is true
>+PASS canRemoveProperty('border-left') is true
>+PASS canSetProperty('border') is true
>+PASS canRemoveProperty('border') is true
>+PASS canSetProperty('border-color') is true
>+PASS canRemoveProperty('border-color') is true
>+PASS canSetProperty('border-style') is true
>+PASS canRemoveProperty('border-style') is true
>+PASS canSetProperty('border-width') is true
>+PASS canRemoveProperty('border-width') is true
>+PASS canSetProperty('background-position') is true
>+PASS canRemoveProperty('background-position') is true
>+PASS canSetProperty('background-repeat') is true
>+PASS canRemoveProperty('background-repeat') is true
>+PASS canSetProperty('border-spacing') is true
>+PASS canRemoveProperty('border-spacing') is true
>+PASS canSetProperty('list-style') is true
>+PASS canRemoveProperty('list-style') is true
>+PASS canSetProperty('margin') is true
>+PASS canRemoveProperty('margin') is true
>+PASS canSetProperty('outline') is true
>+PASS canRemoveProperty('outline') is true
>+PASS canSetProperty('padding') is true
>+PASS canRemoveProperty('padding') is true
>+PASS canSetProperty('background') is true
>+PASS canRemoveProperty('background') is true
>+PASS canSetProperty('overflow') is true
>+PASS canRemoveProperty('overflow') is true
>+PASS canSetProperty('border-radius') is true
>+PASS canRemoveProperty('border-radius') is true
>+PASS canSetProperty('transition') is true
>+PASS canRemoveProperty('transition') is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/css/cssom-remove-important-shorthand-property.html b/LayoutTests/fast/css/cssom-remove-important-shorthand-property.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e888611fb7c6c7c994cb2a9d5ae3c170646ff9a8
>--- /dev/null
>+++ b/LayoutTests/fast/css/cssom-remove-important-shorthand-property.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<script src="../../resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+
>+shorthandProperties = [
>+    "font",
>+    "border-top",
>+    "border-right",
>+    "border-bottom",
>+    "border-left",
>+    "border",
>+    "border-color",
>+    "border-style",
>+    "border-width",
>+    "background-position",
>+    "background-repeat",
>+    "border-spacing",
>+    "list-style",
>+    "margin",
>+    "outline",
>+    "padding",
>+    "background",
>+    "overflow",
>+    "border-radius",
>+    "transition"
>+];
>+
>+element = document.createElement('span');
>+
>+function canSetProperty(propertyName) {
>+    element.style.setProperty(propertyName, 'initial', 'important');
>+    return element.style.getPropertyValue(propertyName) == 'initial';
>+}
>+
>+function canRemoveProperty(propertyName) {
>+    element.style.removeProperty(propertyName);
>+    return element.style.getPropertyValue(propertyName) == '';
>+}
>+
>+description("Tests that shorthand properties can be removed via CSSOM.");
>+
>+for (i = 0; i < shorthandProperties.length; ++i) {
>+    var propertyName = shorthandProperties[i];
>+
>+    shouldBe("canSetProperty('" + propertyName + "')", "true");
>+    shouldBe("canRemoveProperty('" + propertyName + "')", "true");
>+}
>+
>+</script>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 177684: 322231 | 340565 | 340575 | 340609 | 340641 | 340681