<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[206092] trunk/Source/WebInspectorUI</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/206092">206092</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-09-19 04:29:36 -0700 (Mon, 19 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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 <pecoraro@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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsCSSStyleDeclarationTextEditorjs">trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsColorPickerjs">trunk/Source/WebInspectorUI/UserInterface/Views/ColorPicker.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsInlineSwatchjs">trunk/Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (206091 => 206092)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2016-09-19 Joseph Pecoraro <pecoraro@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):
+
</ins><span class="cx"> 2016-09-18 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Uncaught Exception: null is not an object (evaluating 'this.listItemElement.classList')
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsCSSStyleDeclarationTextEditorjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js (206091 => 206092)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -42,6 +42,7 @@
</span><span class="cx"> this._alwaysShowPropertyNames = {};
</span><span class="cx"> this._filterResultPropertyNames = null;
</span><span class="cx"> this._sortProperties = false;
</span><ins>+ this._hasActiveInlineSwatchEditor = false;
</ins><span class="cx">
</span><span class="cx"> this._linePrefixWhitespace = "";
</span><span class="cx">
</span><span class="lines">@@ -771,6 +772,8 @@
</span><span class="cx">
</span><span class="cx"> _updateTextMarkers(nonatomic)
</span><span class="cx"> {
</span><ins>+ console.assert(!this._hasActiveInlineSwatchEditor, "We should never be recreating markers when we an active inline swatch editor.");
+
</ins><span class="cx"> function update()
</span><span class="cx"> {
</span><span class="cx"> this._clearTextMarkers(true);
</span><span class="lines">@@ -854,6 +857,8 @@
</span><span class="cx"> function createSwatch(swatch, marker, valueObject, valueString)
</span><span class="cx"> {
</span><span class="cx"> swatch.addEventListener(WebInspector.InlineSwatch.Event.ValueChanged, this._inlineSwatchValueChanged, this);
</span><ins>+ swatch.addEventListener(WebInspector.InlineSwatch.Event.Activated, this._inlineSwatchActivated, this);
+ swatch.addEventListener(WebInspector.InlineSwatch.Event.Dismissed, this._inlineSwatchDeactivated, this);
</ins><span class="cx">
</span><span class="cx"> let codeMirrorTextMarker = marker.codeMirrorTextMarker;
</span><span class="cx"> let codeMirrorTextMarkerRange = codeMirrorTextMarker.find();
</span><span class="lines">@@ -1325,6 +1330,8 @@
</span><span class="cx">
</span><span class="cx"> _inlineSwatchValueChanged(event)
</span><span class="cx"> {
</span><ins>+ console.assert(this._hasActiveInlineSwatchEditor);
+
</ins><span class="cx"> let swatch = event && event.target;
</span><span class="cx"> console.assert(swatch);
</span><span class="cx"> if (!swatch)
</span><span class="lines">@@ -1343,29 +1350,6 @@
</span><span class="cx">
</span><span class="cx"> function update()
</span><span class="cx"> {
</span><del>- // 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;
-
</del><span class="cx"> // Sometimes we still might find a stale text marker with findMarksAt.
</span><span class="cx"> range = textMarker.find();
</span><span class="cx"> if (!range)
</span><span class="lines">@@ -1387,6 +1371,16 @@
</span><span class="cx"> this._codeMirror.operation(update.bind(this));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ _inlineSwatchActivated()
+ {
+ this._hasActiveInlineSwatchEditor = true;
+ }
+
+ _inlineSwatchDeactivated()
+ {
+ this._hasActiveInlineSwatchEditor = false;
+ }
+
</ins><span class="cx"> _propertyOverriddenStatusChanged(event)
</span><span class="cx"> {
</span><span class="cx"> this._updateTextMarkerForPropertyIfNeeded(event.target);
</span><span class="lines">@@ -1399,6 +1393,15 @@
</span><span class="cx"> if (this._completionController.isShowingCompletions())
</span><span class="cx"> return;
</span><span class="cx">
</span><ins>+ if (this._hasActiveInlineSwatchEditor)
+ return;
+
+ // Don't try to update the document after just modifying a swatch.
+ if (this._ignoreNextPropertiesChanged) {
+ this._ignoreNextPropertiesChanged = false;
+ return;
+ }
+
</ins><span class="cx"> // Reset the content if the text is different and we are not focused.
</span><span class="cx"> if (!this.focused && (!this._style.text || this._style.text !== this._formattedContent())) {
</span><span class="cx"> this._resetContent();
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsColorPickerjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ColorPicker.js (206091 => 206092)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -52,6 +52,8 @@
</span><span class="cx"> this._opacityPattern = "url(Images/Checkers.svg)";
</span><span class="cx">
</span><span class="cx"> this._color = "white";
</span><ins>+
+ this._dontUpdateColor = false;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Public
</span><span class="lines">@@ -103,7 +105,7 @@
</span><span class="cx"> this._opacitySlider.value = color.alpha;
</span><span class="cx"> this._updateSliders(this._colorWheel.rawColor, color);
</span><span class="cx">
</span><del>- delete this._dontUpdateColor;
</del><ins>+ this._dontUpdateColor = false;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> colorWheelColorDidChange(colorWheel)
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsInlineSwatchjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js (206091 => 206092)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -92,6 +92,8 @@
</span><span class="cx">
</span><span class="cx"> if (typeof this._valueEditor.removeListeners === "function")
</span><span class="cx"> this._valueEditor.removeListeners();
</span><ins>+
+ this.dispatchEventToListeners(WebInspector.InlineSwatch.Event.Deactivated);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Private
</span><span class="lines">@@ -163,6 +165,8 @@
</span><span class="cx"> popover.content = this._valueEditor.element;
</span><span class="cx"> popover.present(bounds.pad(2), [WebInspector.RectEdge.MIN_X]);
</span><span class="cx">
</span><ins>+ this.dispatchEventToListeners(WebInspector.InlineSwatch.Event.Activated);
+
</ins><span class="cx"> let value = this._value || this._fallbackValue();
</span><span class="cx"> if (this._type === WebInspector.InlineSwatch.Type.Bezier)
</span><span class="cx"> this._valueEditor.bezier = value;
</span><span class="lines">@@ -297,5 +301,7 @@
</span><span class="cx">
</span><span class="cx"> WebInspector.InlineSwatch.Event = {
</span><span class="cx"> BeforeClicked: "inline-swatch-before-clicked",
</span><del>- ValueChanged: "inline-swatch-value-changed"
</del><ins>+ ValueChanged: "inline-swatch-value-changed",
+ Activated: "inline-swatch-activated",
+ Deactivated: "inline-swatch-deactivated",
</ins><span class="cx"> };
</span></span></pre>
</div>
</div>
</body>
</html>