Title: [206092] trunk/Source/WebInspectorUI
Revision
206092
Author
commit-qu...@webkit.org
Date
2016-09-19 04:29:36 -0700 (Mon, 19 Sep 2016)

Log Message

Web Inspector: Color picker in Style sidebar stops working after 1st color change
https://bugs.webkit.org/show_bug.cgi?id=162115
<rdar://problem/28349875>

Patch by Joseph Pecoraro <pecor...@apple.com> on 2016-09-19
Reviewed by Brian Burg.

* UserInterface/Views/CSSStyleDeclarationTextEditor.js:
(WebInspector.CSSStyleDeclarationTextEditor):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._updateTextMarkers):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._updateTextMarkers.createSwatch):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchActivated):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchDeactivated):
Listen for swatch activated / inactivated events to set some state.

(WebInspector.CSSStyleDeclarationTextEditor.prototype._propertiesChanged):
Do not wipe markers if there is an active inline swatch. That
would break behavior for that active swatch.

(WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchValueChanged):
Eliminate old, incorrect, and now unnecessary code for trying to recover
a textMarker for an inline swatch if the textMarker went away. Besides being
incorrect, if an inline swatch's textMarker goes away, then we will already
have issues, because any active popover will still be connected to the
original marker and swatch element that no longer appear in the editor.

* UserInterface/Views/ColorPicker.js:
(WebInspector.ColorPicker):
(WebInspector.ColorPicker.prototype.set color):
* UserInterface/Views/InlineSwatch.js:
(WebInspector.InlineSwatch.prototype.didDismissPopover):
(WebInspector.InlineSwatch.prototype._swatchElementClicked):

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (206091 => 206092)


--- trunk/Source/WebInspectorUI/ChangeLog	2016-09-19 09:39:56 UTC (rev 206091)
+++ trunk/Source/WebInspectorUI/ChangeLog	2016-09-19 11:29:36 UTC (rev 206092)
@@ -1,3 +1,37 @@
+2016-09-19  Joseph Pecoraro  <pecor...@apple.com>
+
+        Web Inspector: Color picker in Style sidebar stops working after 1st color change
+        https://bugs.webkit.org/show_bug.cgi?id=162115
+        <rdar://problem/28349875>
+
+        Reviewed by Brian Burg.
+
+        * UserInterface/Views/CSSStyleDeclarationTextEditor.js:
+        (WebInspector.CSSStyleDeclarationTextEditor):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._updateTextMarkers):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._updateTextMarkers.createSwatch):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchActivated):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchDeactivated):
+        Listen for swatch activated / inactivated events to set some state.
+
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._propertiesChanged):
+        Do not wipe markers if there is an active inline swatch. That
+        would break behavior for that active swatch.
+
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._inlineSwatchValueChanged):
+        Eliminate old, incorrect, and now unnecessary code for trying to recover
+        a textMarker for an inline swatch if the textMarker went away. Besides being
+        incorrect, if an inline swatch's textMarker goes away, then we will already
+        have issues, because any active popover will still be connected to the
+        original marker and swatch element that no longer appear in the editor.
+
+        * UserInterface/Views/ColorPicker.js:
+        (WebInspector.ColorPicker):
+        (WebInspector.ColorPicker.prototype.set color):
+        * UserInterface/Views/InlineSwatch.js:
+        (WebInspector.InlineSwatch.prototype.didDismissPopover):
+        (WebInspector.InlineSwatch.prototype._swatchElementClicked):
+
 2016-09-18  Joseph Pecoraro  <pecor...@apple.com>
 
         Uncaught Exception: null is not an object (evaluating 'this.listItemElement.classList')

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js (206091 => 206092)


--- trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js	2016-09-19 09:39:56 UTC (rev 206091)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js	2016-09-19 11:29:36 UTC (rev 206092)
@@ -42,6 +42,7 @@
         this._alwaysShowPropertyNames = {};
         this._filterResultPropertyNames = null;
         this._sortProperties = false;
+        this._hasActiveInlineSwatchEditor = false;
 
         this._linePrefixWhitespace = "";
 
@@ -771,6 +772,8 @@
 
     _updateTextMarkers(nonatomic)
     {
+        console.assert(!this._hasActiveInlineSwatchEditor, "We should never be recreating markers when we an active inline swatch editor.");
+
         function update()
         {
             this._clearTextMarkers(true);
@@ -854,6 +857,8 @@
         function createSwatch(swatch, marker, valueObject, valueString)
         {
             swatch.addEventListener(WebInspector.InlineSwatch.Event.ValueChanged, this._inlineSwatchValueChanged, this);
+            swatch.addEventListener(WebInspector.InlineSwatch.Event.Activated, this._inlineSwatchActivated, this);
+            swatch.addEventListener(WebInspector.InlineSwatch.Event.Dismissed, this._inlineSwatchDeactivated, this);
 
             let codeMirrorTextMarker = marker.codeMirrorTextMarker;
             let codeMirrorTextMarkerRange = codeMirrorTextMarker.find();
@@ -1325,6 +1330,8 @@
 
     _inlineSwatchValueChanged(event)
     {
+        console.assert(this._hasActiveInlineSwatchEditor);
+
         let swatch = event && event.target;
         console.assert(swatch);
         if (!swatch)
@@ -1343,29 +1350,6 @@
 
         function update()
         {
-            // The original text marker might have been cleared by a style update,
-            // in this case we need to find the new text marker so we know the
-            // right range for the new style text.
-            if (!textMarker || !textMarker.find()) {
-                textMarker = null;
-
-                let marks = this._codeMirror.findMarksAt(range.from);
-                if (!marks.length)
-                    return;
-
-                for (let mark of marks) {
-                    let type = WebInspector.TextMarker.textMarkerForCodeMirrorTextMarker(mark).type;
-                    if (Object.values(WebInspector.TextMarker.Type).includes(type))
-                        continue;
-
-                    textMarker = mark;
-                    break;
-                }
-            }
-
-            if (!textMarker)
-                return;
-
             // Sometimes we still might find a stale text marker with findMarksAt.
             range = textMarker.find();
             if (!range)
@@ -1387,6 +1371,16 @@
         this._codeMirror.operation(update.bind(this));
     }
 
+    _inlineSwatchActivated()
+    {
+        this._hasActiveInlineSwatchEditor = true;
+    }
+
+    _inlineSwatchDeactivated()
+    {
+        this._hasActiveInlineSwatchEditor = false;
+    }    
+
     _propertyOverriddenStatusChanged(event)
     {
         this._updateTextMarkerForPropertyIfNeeded(event.target);
@@ -1399,6 +1393,15 @@
         if (this._completionController.isShowingCompletions())
             return;
 
+        if (this._hasActiveInlineSwatchEditor)
+            return;
+
+        // Don't try to update the document after just modifying a swatch.
+        if (this._ignoreNextPropertiesChanged) {
+            this._ignoreNextPropertiesChanged = false;
+            return;
+        }
+
         // Reset the content if the text is different and we are not focused.
         if (!this.focused && (!this._style.text || this._style.text !== this._formattedContent())) {
             this._resetContent();

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ColorPicker.js (206091 => 206092)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ColorPicker.js	2016-09-19 09:39:56 UTC (rev 206091)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ColorPicker.js	2016-09-19 11:29:36 UTC (rev 206092)
@@ -52,6 +52,8 @@
         this._opacityPattern = "url(Images/Checkers.svg)";
 
         this._color = "white";
+
+        this._dontUpdateColor = false;
     }
 
     // Public
@@ -103,7 +105,7 @@
         this._opacitySlider.value = color.alpha;
         this._updateSliders(this._colorWheel.rawColor, color);
 
-        delete this._dontUpdateColor;
+        this._dontUpdateColor = false;
     }
 
     colorWheelColorDidChange(colorWheel)

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js (206091 => 206092)


--- trunk/Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js	2016-09-19 09:39:56 UTC (rev 206091)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js	2016-09-19 11:29:36 UTC (rev 206092)
@@ -92,6 +92,8 @@
 
         if (typeof this._valueEditor.removeListeners === "function")
             this._valueEditor.removeListeners();
+
+        this.dispatchEventToListeners(WebInspector.InlineSwatch.Event.Deactivated);
     }
 
     // Private
@@ -163,6 +165,8 @@
         popover.content = this._valueEditor.element;
         popover.present(bounds.pad(2), [WebInspector.RectEdge.MIN_X]);
 
+        this.dispatchEventToListeners(WebInspector.InlineSwatch.Event.Activated);
+
         let value = this._value || this._fallbackValue();
         if (this._type === WebInspector.InlineSwatch.Type.Bezier)
             this._valueEditor.bezier = value;
@@ -297,5 +301,7 @@
 
 WebInspector.InlineSwatch.Event = {
     BeforeClicked: "inline-swatch-before-clicked",
-    ValueChanged: "inline-swatch-value-changed"
+    ValueChanged: "inline-swatch-value-changed",
+    Activated: "inline-swatch-activated",
+    Deactivated: "inline-swatch-deactivated",
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to