Attachment 361262 Details for Bug 194318 – Patch

[patch] Patch

patch.txt (text/plain), 7.95 KB, created by Nikita Vasilyev on 2019-02-05 18:32:16 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Nikita Vasilyev

Created: 2019-02-05 18:32:16 PST

Size: 7.95 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 74908a7fa06..a282865318e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+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  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: Fix modify-css-property-race.html test failures
>diff --git a/LayoutTests/inspector/css/modify-css-property-race-expected.txt b/LayoutTests/inspector/css/modify-css-property-race-expected.txt
>index 139125488ac..aa3c0deace3 100644
>--- a/LayoutTests/inspector/css/modify-css-property-race-expected.txt
>+++ b/LayoutTests/inspector/css/modify-css-property-race-expected.txt
>@@ -3,9 +3,8 @@ Testing that changes to "style" attribute made from page's JavaScript are ignore
> 
> == 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.
> 
>diff --git a/LayoutTests/inspector/css/modify-css-property-race.html b/LayoutTests/inspector/css/modify-css-property-race.html
>index f72b4f020ee..d899b6349b1 100644
>--- a/LayoutTests/inspector/css/modify-css-property-race.html
>+++ b/LayoutTests/inspector/css/modify-css-property-race.html
>@@ -7,12 +7,11 @@ let requestAnimationFrameID;
> 
> 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 @@ function test() {
>             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 @@ function test() {
> </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>
>diff --git a/LayoutTests/inspector/css/modify-css-property.html b/LayoutTests/inspector/css/modify-css-property.html
>index 32ffd9a7aa3..69fc80815bc 100644
>--- a/LayoutTests/inspector/css/modify-css-property.html
>+++ b/LayoutTests/inspector/css/modify-css-property.html
>@@ -117,7 +117,7 @@ function test() {
> 
>             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.`);
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 1c4338aceaf..33b92a48582 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,15 @@
>+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>
> 
>         Web Inspector: Lots of time spent updating related resources in ResourceDetailsSidebar when loading a page with lots of resources
>diff --git a/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js b/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>index 6d06c8a7d23..886f9d844b7 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>@@ -173,8 +173,9 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
>         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()

Actions: View | Formatted Diff | Diff

Attachments on bug 194318: 361249 | 361260 | 361262