Attachment 372308 Details for Bug 198505 – Patch

[patch] Patch

patch.txt (text/plain), 7.00 KB, created by Nikita Vasilyev on 2019-06-17 18:03:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Nikita Vasilyev

Created: 2019-06-17 18:03:07 PDT

Size: 7.00 KB

patch

obsolete

>diff --git a/LayoutTests/inspector/css/modify-css-property.html b/LayoutTests/inspector/css/modify-css-property.html
>index 69fc80815bc..32ffd9a7aa3 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();
> 
>-            WI.CSSStyleDeclaration.awaitEvent(WI.CSSStyleDeclaration.Event.PropertiesChanged).then((event) => {
>+            styleDeclaration.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/LayoutTests/inspector/css/pseudo-element-matches-for-pseudo-element-node.html b/LayoutTests/inspector/css/pseudo-element-matches-for-pseudo-element-node.html
>index 3189f2af0a0..002e1d02e46 100644
>--- a/LayoutTests/inspector/css/pseudo-element-matches-for-pseudo-element-node.html
>+++ b/LayoutTests/inspector/css/pseudo-element-matches-for-pseudo-element-node.html
>@@ -34,7 +34,7 @@ function test() {
>             }
>         }
> 
>-        InspectorTest.completeTest();                
>+        InspectorTest.completeTest();
>     }
> 
>     WI.domManager.requestDocument(function(documentNode) {
>@@ -46,7 +46,8 @@ function test() {
>             InspectorTest.log("Refreshing styles for ::before pseudo element");
>             nodeStyles = WI.cssManager.stylesForNode(beforePseudoElementDOMNode);
>             nodeStyles.addEventListener(WI.DOMNodeStyles.Event.Refreshed, validateStyles);
>-            nodeStyles.refresh();
>+            if (!nodeStyles.pendingRefresh)
>+                nodeStyles.refresh();
>         });
>     });
> 
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 926b90cc677..1802379ed8d 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-06-08  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        REGRESSION(r240946): Web Inspector: Styles: Pasting multiple properties has issues
>+        https://bugs.webkit.org/show_bug.cgi?id=198505
>+        <rdar://problem/51374780>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since r240946, setting WI.CSSStyleDeclaration.prototype.text updates the text immediately.
>+        When WI.CSSStyleDeclaration.prototype.update gets called after setting text, it exits early
>+        without firing WI.CSSStyleDeclaration.Event.PropertiesChanged.
>+
>+        This patch adds _pendingPropertiesChanged flag to fire WI.CSSStyleDeclaration.Event.PropertiesChanged
>+        after changing text.
>+
>+        * UserInterface/Models/CSSStyleDeclaration.js:
>+        (WI.CSSStyleDeclaration):
>+        (WI.CSSStyleDeclaration.prototype.set text):
>+
> 2019-06-13  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: REGRESSION(r246178): extra spaces added in at-rules when formatting CSS
>diff --git a/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js b/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>index e03160439f0..8f2e9998d99 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
>@@ -42,6 +42,7 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
> 
>         this._initialState = null;
>         this._updatesInProgressCount = 0;
>+        this._pendingPropertiesChanged = false;
>         this._locked = false;
>         this._pendingProperties = [];
>         this._propertyNameMap = {};
>@@ -183,9 +184,14 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
> 
>         // 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)
>+        if (oldText === this._text && !this._pendingPropertiesChanged && this._type !== WI.CSSStyleDeclaration.Type.Computed)
>             return;
> 
>+        this._pendingPropertiesChanged = false;
>+
>+        if (this._text === "width: 100px")
>+            console.trace();
>+
>         function delayed()
>         {
>             this.dispatchEventToListeners(WI.CSSStyleDeclaration.Event.PropertiesChanged);
>@@ -237,6 +243,7 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
>             clearTimeout(timeoutId);
>             timeoutId = null;
>             this._updatesInProgressCount = Math.max(0, this._updatesInProgressCount - 1);
>+            this._pendingPropertiesChanged = true;
>         };
> 
>         this._nodeStyles.changeStyleText(this, text, styleTextDidChange);
>diff --git a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>index 15e39c9e28a..1eb22401fb6 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>@@ -45,6 +45,10 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
> 
>         this._propertyNameToEffectivePropertyMap = {};
> 
>+        this._refreshDebouncer = new Debouncer(() => {
>+            this.refresh();
>+        });
>+
>         this._pendingRefreshTask = null;
>         this.refresh();
>     }
>@@ -133,6 +137,11 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>         return this._pendingRefreshTask || this._needsRefresh;
>     }
> 
>+    get pendingRefresh()
>+    {
>+        return !!this._pendingRefreshTask;
>+    }
>+
>     refreshIfNeeded()
>     {
>         if (this._pendingRefreshTask)
>@@ -144,10 +153,12 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
> 
>     refresh()
>     {
>-        if (this._pendingRefreshTask)
>+        if (this._pendingRefreshTask) {
>+            // A promise in progress cannot be canceled. Refresh again after the current refresh is done.
>+            this._needsRefresh = true;
>             return this._pendingRefreshTask;
>-
>-        this._needsRefresh = false;
>+        } else
>+            this._needsRefresh = false;
> 
>         let previousStylesMap = this._stylesMap.copy();
> 
>@@ -318,6 +329,10 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>         this._pendingRefreshTask = Promise.all([fetchedMatchedStylesPromise.promise, fetchedInlineStylesPromise.promise, fetchedComputedStylesPromise.promise])
>         .then(() => {
>             this._pendingRefreshTask = null;
>+
>+            if (this._needsRefresh)
>+                this.refresh();
>+
>             return this;
>         });
> 
>@@ -503,7 +518,7 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>             }
> 
>             callback();
>-            this.refresh();
>+            this._refreshDebouncer.delayForTime(100);
>         };
> 
>         CSSAgent.setStyleText(style.id, text, didSetStyleText);

Flags:

nvasilyev: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 198505: 371683 | 371688 | 371690 | 371694 | 371783 | 371814 | 372003 | 372011 | 372203 | 372208 | 372308 | 372318 | 372355 | 372362 | 372394