Title: [241011] trunk
Revision
241011
Author
[email protected]
Date
2019-02-05 22:54:43 -0800 (Tue, 05 Feb 2019)

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

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (241010 => 241011)


--- trunk/LayoutTests/ChangeLog	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/LayoutTests/ChangeLog	2019-02-06 06:54:43 UTC (rev 241011)
@@ -1,3 +1,15 @@
+2019-02-05  Nikita Vasilyev  <[email protected]>
+
+        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  <[email protected]>
 
         [iOS] Layout tests editing/pasteboard/smart-paste-007.html and editing/pasteboard/smart-paste-008.html are failing

Modified: trunk/LayoutTests/inspector/css/modify-css-property-race-expected.txt (241010 => 241011)


--- trunk/LayoutTests/inspector/css/modify-css-property-race-expected.txt	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/LayoutTests/inspector/css/modify-css-property-race-expected.txt	2019-02-06 06:54:43 UTC (rev 241011)
@@ -3,9 +3,8 @@
 
 == 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.
 

Modified: trunk/LayoutTests/inspector/css/modify-css-property-race.html (241010 => 241011)


--- trunk/LayoutTests/inspector/css/modify-css-property-race.html	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/LayoutTests/inspector/css/modify-css-property-race.html	2019-02-06 06:54:43 UTC (rev 241011)
@@ -7,12 +7,11 @@
 
 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");
 
@@ -63,29 +62,28 @@
             InspectorTest.evaluateInPage("expand()");
             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()");
                     WI.CSSStyleDeclaration.removeEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
                     resolve();
                 }
-                ++updateCount;
             }
 
-            WI.CSSStyleDeclaration.addEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
+            let style = getInlineStyleDeclaration();
+            style.addEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, styleDecorationUpdated);
         }
     });
 
@@ -112,6 +110,6 @@
 </head>
 <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>

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


--- trunk/LayoutTests/inspector/css/modify-css-property.html	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/LayoutTests/inspector/css/modify-css-property.html	2019-02-06 06:54:43 UTC (rev 241011)
@@ -117,7 +117,7 @@
 
             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".`);
                 InspectorTest.expectEqual(styleDeclaration.text, `width: 200px;`, `Inline style declaration text should update when not locked.`);

Modified: trunk/Source/WebInspectorUI/ChangeLog (241010 => 241011)


--- trunk/Source/WebInspectorUI/ChangeLog	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/Source/WebInspectorUI/ChangeLog	2019-02-06 06:54:43 UTC (rev 241011)
@@ -1,3 +1,15 @@
+2019-02-05  Nikita Vasilyev  <[email protected]>
+
+        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  <[email protected]>
 
         Web Inspector: Lots of time spent updating related resources in ResourceDetailsSidebar when loading a page with lots of resources

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (241010 => 241011)


--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2019-02-06 05:52:01 UTC (rev 241010)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js	2019-02-06 06:54:43 UTC (rev 241011)
@@ -173,8 +173,9 @@
         if (dontFireEvents)
             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;
 
         function delayed()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to