Changeset 246816 in webkit

Timestamp:

Jun 25, 2019 4:40:14 PM (5 years ago)

Author:

Nikita Vasilyev

Message:

REGRESSION(r246621): Web Inspector: Styles: property may get removed when editing after deleting value
​https://bugs.webkit.org/show_bug.cgi?id=199143
<rdar://problem/52042815>

Reviewed by Matt Baker.

r246621 affected when WI.CSSStyleDeclaration.Event.PropertiesChanged event is fired, which caused
SpreadsheetCSSStyleDeclarationEditor to possibly enter a state when focused property is
incorrectly set to false.

• UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:

(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyRemoved): Removed.
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyWillRemove): Added.
Renamed from "removed" to "willRemove" since we need to call it before the property DOM element
is detached from the document.

• UserInterface/Views/SpreadsheetStyleProperty.js:

(WI.SpreadsheetStyleProperty.prototype.remove):
Don't set focused to false when the removed property wasn't focused.
This code need to exist because removing focused element from the DOM doesn't trigger blur event.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js (modified) (2 diffs)
• UserInterface/Views/SpreadsheetStyleProperty.js (modified) (2 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-06-25  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        REGRESSION(r246621): Web Inspector: Styles: property may get removed when editing after deleting value
+        https://bugs.webkit.org/show_bug.cgi?id=199143
+        <rdar://problem/52042815>
+
+        Reviewed by Matt Baker.
+
+        r246621 affected when WI.CSSStyleDeclaration.Event.PropertiesChanged event is fired, which caused
+        SpreadsheetCSSStyleDeclarationEditor to possibly enter a state when `focused` property is
+        incorrectly set to false.
+
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyRemoved): Removed.
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyWillRemove): Added.
+        Renamed from "removed" to "willRemove" since we need to call it before the property DOM element
+        is detached from the document.
+
+        * UserInterface/Views/SpreadsheetStyleProperty.js:
+        (WI.SpreadsheetStyleProperty.prototype.remove):
+        Don't set focused to false when the removed property wasn't focused.
+        This code need to exist because removing focused element from the DOM doesn't trigger blur event.
+
 2019-06-25  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js

@@ -488 +488 @@
     }
 
-    spreadsheetStylePropertyRemoved(propertyView)
+    spreadsheetStylePropertyWillRemove(propertyView)
     {
         this._propertyViews.remove(propertyView);
@@ -495 +495 @@
             this._propertyViews[index].index = index;
 
-        this.focused = false;
+        let wasFocused = document.activeElement && propertyView.element.contains(document.activeElement);
+        if (wasFocused)
+            this.focused = false;
     }
 

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js

@@ -139 +139 @@
     remove(replacement = null)
     {
+        if (this._delegate && typeof this._delegate.spreadsheetStylePropertyWillRemove === "function")
+            this._delegate.spreadsheetStylePropertyWillRemove(this);
+
         this.element.remove();
 
@@ -147 +150 @@
 
         this.detached();
-
-        if (this._delegate && typeof this._delegate.spreadsheetStylePropertyRemoved === "function")
-            this._delegate.spreadsheetStylePropertyRemoved(this);
     }