Changeset 241011 in webkit

Timestamp:

Feb 5, 2019 10:54:43 PM (5 years ago)

Author:

Nikita Vasilyev

Message:

Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty
​https://bugs.webkit.org/show_bug.cgi?id=194318

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Previously, WI.CSSStyleDeclaration.Event.PropertiesChanged fired when
old text and new text were empty strings.

• UserInterface/Models/CSSStyleDeclaration.js:

LayoutTests:

Fix the flaky test on Debug.

• inspector/css/modify-css-property-race-expected.txt:
• inspector/css/modify-css-property-race.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/css/modify-css-property-race-expected.txt (modified) (1 diff)
• LayoutTests/inspector/css/modify-css-property-race.html (modified) (4 diffs)
• LayoutTests/inspector/css/modify-css-property.html (modified) (1 diff)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-02-05  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty
+        https://bugs.webkit.org/show_bug.cgi?id=194318
+
+        Reviewed by Devin Rousso.
+
+        Fix the flaky test on Debug.
+
+        * inspector/css/modify-css-property-race-expected.txt:
+        * inspector/css/modify-css-property-race.html:
+
 2019-02-05  Megan Gardner  <megan_gardner@apple.com>
 

trunk/LayoutTests/inspector/css/modify-css-property-race-expected.txt

@@ -4 +4 @@
 == Running test suite: ModifyCSSProperty
 -- Running test case: ModifyCSSPropertyRace.ChangeInlineStyle
-PASS: Height should be greater than 42px.
-PASS: Height should be greater than 42px.
-PASS: Height should be 10px.
-PASS: CSSStyleDeclaration text should update.
-PASS: Height should be greater or equal 10px.
+PASS: Height should have increased from 10px.
+PASS: Height should have increased from 11px.
+PASS: Height should be 100px or more.
+PASS: Height should be 101px or more.
 

trunk/LayoutTests/inspector/css/modify-css-property-race.html

@@ -8 +8 @@
 function expand() {
     let element = document.getElementById("x");
-    let initialHeight = parseInt(element.style.height);
     let i = 0;
 
     let loop = () => {
         ++i;
-        let height = initialHeight + i;
+        let height = parseInt(element.style.height) + 1;
         element.style.height = height + "px";
         TestPage.dispatchEventToFrontend("TestPage-styleChanged", height + "px");
@@ -64 +63 @@
             let updateCount = 0;
 
-            function styleDecorationUpdated(event) {
-                if (!updateCount) {
-                    let valueNumber = parseInt(getProperty("height").rawValue);
-                    InspectorTest.expectGreaterThan(valueNumber, 42, "Height should be greater than 42px.");
-                } else if (updateCount === 1) {
-                    let valueNumber = parseInt(getProperty("height").rawValue);
-                    InspectorTest.expectGreaterThan(valueNumber, 42, "Height should be greater than 42px.");
-                    getProperty("height").rawValue = "10px";
-                } else if (updateCount === 2) {
-                    InspectorTest.expectEqual(getProperty("height").rawValue, "10px", "Height should be 10px.");
-                    InspectorTest.expectEqual(getInlineStyleDeclaration().text, "height: 10px;", "CSSStyleDeclaration text should update.");
-                } else {
-                    let valueNumber = parseInt(getProperty("height").rawValue);
-                    InspectorTest.expectGreaterThanOrEqual(valueNumber, 10, "Height should be greater or equal 10px.");
+            function styleDecorationUpdated() {
+                ++updateCount;
+                let heightNumber = parseInt(getProperty("height").rawValue);
+
+                if (updateCount === 1)
+                    InspectorTest.expectGreaterThan(heightNumber, 10, "Height should have increased from 10px.");
+                else if (updateCount === 2) {
+                    InspectorTest.expectGreaterThan(heightNumber, 11, "Height should have increased from 11px.");
+                    getProperty("height").rawValue = "100px";
+                } else if (updateCount === 3)
+                    InspectorTest.expectGreaterThanOrEqual(heightNumber, 100, "Height should be 100px or more.");
+                else {
+                    InspectorTest.expectGreaterThanOrEqual(heightNumber, 101, "Height should be 101px or more.");
 
                     InspectorTest.evaluateInPage("stopExpanding()");
@@ -83 +81 @@
                     resolve();
                 }
-                ++updateCount;
             }
 
-            WI.CSSStyleDeclaration.addEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
+            let style = getInlineStyleDeclaration();
+            style.addEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
         }
     });
@@ -113 +111 @@
 <body onload="runTest()">
     <p>Testing that changes to "style" attribute made from page's JavaScript are ignored when there's a pending change made via Web Inspector.</p>
-    <div id="x" style="height: 42px"></div>
+    <div id="x" style="height: 10px"></div>
 </body>
 </html>

trunk/LayoutTests/inspector/css/modify-css-property.html

@@ -118 +118 @@
             let styleDeclaration = getInlineStyleDeclaration();
 
-            styleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged).then((event) => {
+            WI.CSSStyleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged).then((event) => {
                 InspectorTest.expectThat(!styleDeclaration.locked, `Style declaration is unlocked.`);
                 InspectorTest.expectEqual(getProperty("width").rawValue, "200px", `"width" property value should update to "200px".`);

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-02-05  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty
+        https://bugs.webkit.org/show_bug.cgi?id=194318
+
+        Reviewed by Devin Rousso.
+
+        Previously, WI.CSSStyleDeclaration.Event.PropertiesChanged fired when
+        old text and new text were empty strings.
+
+        * UserInterface/Models/CSSStyleDeclaration.js:
+
 2019-02-05  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js

@@ -174 +174 @@
             return;
 
-        // Don't fire the event if there is text and it hasn't changed.
-        if (oldText && this._text && oldText === this._text)
+        // Don't fire the event if text hasn't changed. However, it should still fire for Computed style declarations
+        // because it never has text.
+        if (oldText === this._text && this._type !== WI.CSSStyleDeclaration.Type.Computed)
             return;