Title: [287891] trunk
Revision
287891
Author
nvasil...@apple.com
Date
2022-01-11 10:53:40 -0800 (Tue, 11 Jan 2022)

Log Message

REGRESSION (r283723): Web Inspector: CSS declarations unexpectedly removed when editing property value
https://bugs.webkit.org/show_bug.cgi?id=233195

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Re-attach CSS property if it was detached while editing.

CSSProperty is detached when focusing on property name and deleting it. Consequent edits of the detached
CSSProperty were not saved. This patch re-attaches detached property at the previous position.

* UserInterface/Models/CSSProperty.js:
(WI.CSSProperty.prototype.set name):
* UserInterface/Models/CSSStyleDeclaration.js:
(WI.CSSStyleDeclaration.prototype.newBlankProperty):
(WI.CSSStyleDeclaration.prototype.insertProperty):
Introduce this method since the logic is used in two different places now.

LayoutTests:

Test removing CSS property name.

* inspector/css/modify-css-property-expected.txt:
* inspector/css/modify-css-property.html:
* inspector/css/resources/modify-css-property.css:
(.rule-e):

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (287890 => 287891)


--- trunk/LayoutTests/ChangeLog	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/LayoutTests/ChangeLog	2022-01-11 18:53:40 UTC (rev 287891)
@@ -1,3 +1,17 @@
+2022-01-11  Nikita Vasilyev  <nvasil...@apple.com>
+
+        REGRESSION (r283723): Web Inspector: CSS declarations unexpectedly removed when editing property value
+        https://bugs.webkit.org/show_bug.cgi?id=233195
+
+        Reviewed by Devin Rousso.
+
+        Test removing CSS property name.
+
+        * inspector/css/modify-css-property-expected.txt:
+        * inspector/css/modify-css-property.html:
+        * inspector/css/resources/modify-css-property.css:
+        (.rule-e):
+
 2022-01-11  Rob Buis  <rb...@igalia.com>
 
         [CSS contain] Update css-contain tests from WPT

Modified: trunk/LayoutTests/inspector/css/modify-css-property-expected.txt (287890 => 287891)


--- trunk/LayoutTests/inspector/css/modify-css-property-expected.txt	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/LayoutTests/inspector/css/modify-css-property-expected.txt	2022-01-11 18:53:40 UTC (rev 287891)
@@ -41,3 +41,7 @@
 PASS: Style declaration text should update immediately with commented out property.
 PASS: Uncommented property should be enabled.
 
+-- Running test case: ModifyCSSProperty.ReplacePropertyName
+PASS: Style declaration text should be empty.
+PASS: Style declaration text should have new property name.
+

Modified: trunk/LayoutTests/inspector/css/modify-css-property.html (287890 => 287891)


--- trunk/LayoutTests/inspector/css/modify-css-property.html	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/LayoutTests/inspector/css/modify-css-property.html	2022-01-11 18:53:40 UTC (rev 287891)
@@ -316,6 +316,35 @@
         }
     });
 
+    suite.addTestCase({
+        name: "ModifyCSSProperty.ReplacePropertyName",
+        async test() {
+            let getMatchedStyleDeclaration = () => {
+                for (let rule of nodeStyles.matchedRules) {
+                    if (rule.selectorText === ".rule-e")
+                        return rule.style;
+                }
+                throw "No declaration found.";
+            };
+            let getProperty = (propertyName) => {
+                let styleDeclaration = getMatchedStyleDeclaration();
+                for (let property of styleDeclaration.properties) {
+                    if (property.name === propertyName)
+                        return property;
+                }
+                throw "No property found.";
+            };
+            let styleDeclaration = getMatchedStyleDeclaration();
+
+            let cssProperty = getProperty("color");
+            cssProperty.name = "";
+            InspectorTest.expectEqual(styleDeclaration.text, "", "Style declaration text should be empty.");
+
+            cssProperty.name = "border-color";
+            InspectorTest.expectEqual(styleDeclaration.text, `\n    border-color: darkseagreen;\n`, "Style declaration text should have new property name.");
+        }
+    });
+
     WI.domManager.requestDocument((documentNode) => {
         documentNode.querySelector("#x", (contentNodeId) => {
             if (contentNodeId) {
@@ -339,6 +368,6 @@
 </head>
 <body _onload_="runTest()">
     <p>Testing that CSSStyleDeclaration update immediately after modifying its properties when it is not locked.</p>
-    <div id="x" class="test-node rule-a rule-b rule-c rule-d" style="width: 100px"></div>
+    <div id="x" class="test-node rule-a rule-b rule-c rule-d rule-e" style="width: 100px"></div>
 </body>
 </html>

Modified: trunk/LayoutTests/inspector/css/resources/modify-css-property.css (287890 => 287891)


--- trunk/LayoutTests/inspector/css/resources/modify-css-property.css	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/LayoutTests/inspector/css/resources/modify-css-property.css	2022-01-11 18:53:40 UTC (rev 287891)
@@ -13,3 +13,6 @@
     /* padding-right: 0px; */
 }
 .rule-d {/*font-size: 13px;*//*border: 2px solid brown*/}
+.rule-e {
+    color: darkseagreen;
+}

Modified: trunk/Source/WebInspectorUI/ChangeLog (287890 => 287891)


--- trunk/Source/WebInspectorUI/ChangeLog	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/Source/WebInspectorUI/ChangeLog	2022-01-11 18:53:40 UTC (rev 287891)
@@ -1,3 +1,22 @@
+2022-01-11  Nikita Vasilyev  <nvasil...@apple.com>
+
+        REGRESSION (r283723): Web Inspector: CSS declarations unexpectedly removed when editing property value
+        https://bugs.webkit.org/show_bug.cgi?id=233195
+
+        Reviewed by Devin Rousso.
+
+        Re-attach CSS property if it was detached while editing.
+
+        CSSProperty is detached when focusing on property name and deleting it. Consequent edits of the detached
+        CSSProperty were not saved. This patch re-attaches detached property at the previous position.
+
+        * UserInterface/Models/CSSProperty.js:
+        (WI.CSSProperty.prototype.set name):
+        * UserInterface/Models/CSSStyleDeclaration.js:
+        (WI.CSSStyleDeclaration.prototype.newBlankProperty):
+        (WI.CSSStyleDeclaration.prototype.insertProperty):
+        Introduce this method since the logic is used in two different places now.
+
 2022-01-10  Nikita Vasilyev  <nvasil...@apple.com>
 
         Web Inspector: Increase padding around icons in Alignment editor

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (287890 => 287891)


--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2022-01-11 18:53:40 UTC (rev 287891)
@@ -256,6 +256,17 @@
         if (name === this._name)
             return;
 
+        if (!name) {
+            // Deleting property name causes CSSProperty to be detached from CSSStyleDeclaration.
+            console.assert(!isNaN(this._index), this);
+            this._indexBeforeDetached = this._index;
+        } else if (!isNaN(this._indexBeforeDetached) && isNaN(this._index)) {
+            // Reattach CSSProperty.
+            console.assert(!this._ownerStyle.properties.includes(this), this);
+            this._ownerStyle.insertProperty(this, this._indexBeforeDetached);
+            this._indexBeforeDetached = NaN;
+        }
+
         this._markModified();
         this._name = name;
         this._updateStyleText();

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (287890 => 287891)


--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2022-01-11 18:46:45 UTC (rev 287890)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2022-01-11 18:53:40 UTC (rev 287891)
@@ -390,11 +390,7 @@
 
         this.markModified();
         let property = new WI.CSSProperty(propertyIndex, text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange);
-
-        this._properties.insertAtIndex(property, propertyIndex);
-        for (let index = propertyIndex + 1; index < this._properties.length; index++)
-            this._properties[index].index = index;
-
+        this.insertProperty(property, propertyIndex);
         this.update(this._text, this._properties, this._styleSheetTextRange, {dontFireEvents: true, forceUpdate: true});
 
         return property;
@@ -422,6 +418,17 @@
         WI.cssManager.addModifiedStyle(this);
     }
 
+    insertProperty(cssProperty, propertyIndex)
+    {
+        this._properties.insertAtIndex(cssProperty, propertyIndex);
+        for (let index = propertyIndex + 1; index < this._properties.length; index++)
+            this._properties[index].index = index;
+
+        // Invalidate cached properties.
+        this._enabledProperties = null;
+        this._visibleProperties = null;
+    }
+
     removeProperty(cssProperty)
     {
         // cssProperty.index could be set to NaN by WI.CSSStyleDeclaration.prototype.update.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to